On Tue, Aug 15, 2017 at 2:57 PM Toke Eskildsen <[email protected]> wrote: > Dawid Weiss <[email protected]> wrote: > > Erick's explanation is perfect I think. I would only add that an > issue/patch > > combination will typically bring more eyes to the code. If you seek > > feedback and want to ensure the changes are not breaking things it is > > the way to go. > > Thank you, and thank you Erick. I am a bit surprised that uploading a > patch is the preferred way, at least by you two. But I guess most > committers have scripts in place to ease download/diff/apply? >
I don't have such scripts :-) If I see a .patch file I want to review, I usually review it by itself (don't download or apply). Of course this kinda sucks for a bunch of obvious reasons. > I really like the GitHub pull-request-mechanism, but as I am the one > asking for review of my code (in this case at least), I will of course use > the method with the highest chance of getting a review. I certainly favor GitHub pull requests way more than a patch, in terms of the reviewer experience. I suppose I tend to not do this myself for my own issues only out of habit. If there ultimately isn't any review, no path is particularly easier than the other, I think. I should try to do GitHub PRs more... though I wonder if it will result in more annoying dev list traffic? Related to that I am unsure about Affect/Fix versions in JIRA. The > SOLR-11240 issue is present in Solr 5+, so I just picked the latest > released minor version for 5 & 6 + master. Was that correct? > We just had some email thread(s) about these fields on the dev list. The "affects version" doesn't get much scrutiny... how you filled it seems fine. Now the "fix version/s" matters more. I believe 5.x is very EOL so simply don't put any 5.x version in "fix version/s" unless you really truly want to commit it there despite the EOL status. 6.6 was wrong because 6.6 shipped already. I think 6.7 is appropriate since this is a minor improvement you're talking about here. Again, putting any 6.x version here implies you are going to backport it. And you missed 7.x since most new features will get committed to this version series, in addition to master which you already have listed. *Essentially for every branch you are going to commit this to, there should be a corresponding fix-version. *The actual branch name and JIRA "version" values aren't identically aligned but they are similar. ~ David -- Lucene/Solr Search Committer, Consultant, Developer, Author, Speaker LinkedIn: http://linkedin.com/in/davidwsmiley | Book: http://www.solrenterprisesearchserver.com
