Any plan to merge outstanding existing pull request before starting yet another project?

Question

Any plan to merge outstanding existing pull request before starting yet another project?

bitsofinfo opened this issue 9 years ago · 10 comments

Here is one I submitted last summer which has still to be evaluated/merged.

Answer 1 · 2014-12-18T10:59:14.000Z

hey @bitsofinfo, sorry that this pr has been neglected for so long..would you mind applying it to this repo and assigning it to me? I'll do a review asap so that we can set it up for merging.

Answer 2 · 2014-12-18T22:24:47.000Z

@jsvd is that necessary? According to Jordon's comment @ elastic/logstash-contrib#79 it sounds like it could just be merged from the old contrib project. I was looking at the code diffs last night and its looks like this new logstash-filter-cipher code is a straight copy from the contrib project.

Answer 3 · 2014-12-19T12:05:07.000Z

Indeed it can, I was just going to suggest first some minor cleanups regarding spacing in that PR, since it seems to have tabs in there, that misalign all the code (examples here, here and here).

Also having just if @iv_random_length instead of if !@iv_random_length.nil? is, for most cases, equivalent and would simplify the code a lot since there are many checks like this in the code.

That aside, the PR looks good and we're good to merge.

Answer 4 · 2014-12-19T16:34:21.000Z

I'll move the PR here so we can iterate on it.

Answer 5 · 2014-12-20T20:57:51.000Z

Please do, then I'll fork it again and fix whatever is required. Thanks

Answer 6 · 2014-12-22T12:16:38.000Z

@bitsofinfo done

Answer 7 · 2015-01-05T18:00:07.000Z

thanks, I'll hopefully be able to get to this, this week

Answer 8 · 2015-01-27T18:22:43.000Z

This pull request now incorporates all the changes #3

Answer 9 · 2015-03-20T20:00:28.000Z

I'm also waiting on this pull request. A static IV is a big no-no for encryption.

Answer 10 · 2015-12-09T23:48:36.000Z

All relevant stuff is in referenced PRs above