Andy, Sorry, i haven't responded to this thread in over a week, but I think it's important to keep going.
I just clicked "Cancel Patch" on one of my ticket that has a patch available to see which state it returned to. It did in fact go back to Open. Which I agree is less than ideal. Though we could certainly have a process by which we change the status to "In Progress" after canceling the patch. I guess where my viewpoint differs from yours is in the meaning of "In Review." Let's say that you submit a patch for a JIRA. I then review it and find that it needs some work - let's say there's an issue with licensing not being properly accounted for, for instance. At that point, I no longer consider the patch that you provided to be "In Review." I believe the patch should be canceled, and you will need to submit a new patch. I guess that I view a patch as being an immutable entity. On Feb 24, 2017, at 7:26 PM, Andy LoPresto <alopre...@apache.org<mailto:alopre...@apache.org>> wrote: Mark, Your understanding of “Patch Available” certainly makes sense and it explains why you approach the process the way you do. I have a slightly different personal understanding of “Patch Available” — I read it to mean “the person responsible for this Jira has contributed code they feel solves the issue.” A review will (hopefully) determine if that assertion is correct and complete. I think we kind of agree on "my viewpoint is simply that "Patch Available" means "Awaiting Review" or "In Review.”” but I see “In Review” as a potentially iterative process — it could be on the second pass of the contributor responding to comments, but it’s still “In Review” in my eyes. I don’t know that the granularity of Jira supports the specific workflow states of “been reviewed once but not complete/accepted yet”. What state does “Cancel Patch” result in? If it just reverts to “Open”, I don’t see the value because that obfuscates the difference between a Jira that hasn’t even been touched and one that has 90% of the code done. I agree we should make the RM’s job easier, but I also think it doesn’t help the visibility for reviewers to see a Jira marked as “open” when there is the potential for that patch to be ready for merge in a very short amount of time. I think these conversations will ultimately help us narrow in on shared definitions that make sense to everyone though, so I’m glad we’re talking about it. Andy LoPresto alopre...@apache.org<mailto:alopre...@apache.org> alopresto.apa...@gmail.com<mailto:alopresto.apa...@gmail.com> PGP Fingerprint: 70EC B3E5 98A6 5A3F D3C4 BACE 3C6E F65B 2F7D EF69 On Feb 24, 2017, at 1:07 PM, Mark Payne <marka...@hotmail.com<mailto:marka...@hotmail.com>> wrote: Andy, If the reviewer is looking for clarification, then it may make sense to leave the JIRA in "Patch Available" state as you suggest. If there are minor fixes needed, though, then the patch is not ready. In JIRA, the verbiage for Cancel Patch says "The patch is not yet ready to be committed." So if minor fixes are needed, then I believe it is appropriate to Cancel Patch. Once those changes (minor or not) are made and the PR updated, then the PR needs review again and the status should be changed back to "Patch Available" again. I guess my viewpoint is simply that "Patch Available" means "Awaiting Review" or "In Review." If it is awaiting changes of some kind and won't be merged as-is, then we should Cancel Patch. Do you or others have differing views on the meaning of "Patch Available"? Thanks -Mark On Feb 24, 2017, at 3:27 PM, Andy LoPresto <alopre...@apache.org<mailto:alopre...@apache.org><mailto:alopre...@apache.org>> wrote: Mark, I like your point about updating the Jira with the Fix Version at the time the PR review begins (or when the PR is submitted, if the contributor is aware of this process). I think it’s better than waiting for the merge, as I proposed before. I agree that the reviewer is responsible for keeping the Jira updated in line with their work. I don’t know if I am on the same page as you for “Cancel Patch” if the PR needs changes; sometimes these are minor fixes or just looking for clarification from the contributor, and I don’t think that warrants canceling the availability of the patch. If they are major architectural changes, then that makes more sense to me. Andy LoPresto alopre...@apache.org<mailto:alopre...@apache.org><mailto:alopre...@apache.org> alopresto.apa...@gmail.com<mailto:alopresto.apa...@gmail.com><mailto:alopresto.apa...@gmail.com> PGP Fingerprint: 70EC B3E5 98A6 5A3F D3C4 BACE 3C6E F65B 2F7D EF69 On Feb 24, 2017, at 12:08 PM, Mark Payne <marka...@hotmail.com<mailto:marka...@hotmail.com><mailto:marka...@hotmail.com>> wrote: Personally, I am afraid that if we don't set a Fix Version on JIRA's, that some PR's will be lost or stalled. I rarely go to github and start looking through the PRs. Instead, I go to JIRA and look at what is assigned with a fixVersion of the next release. Then I'll go and review JIRA's that are in a state of "Patch Available." Even then I often come across many PR's that have already been reviewed by one or more other committers and are awaiting updates. So I propose that we address this slightly differently. I believe that we should assign a Fix Version to a JIRA whenever a PR is submitted. Then, whenever a committer reviews a PR, he/she should be responsible for updating the JIRA. If the PR is merged then the JIRA should be resolved as Fixed. But if the PR is not merged because some changes are needed, the reviewer should then go back to the JIRA and click 'Cancel Patch'. We are typically very good about resolving as fixed once a PR is merged, but we don't typically cancel the patch otherwise. If we followed this workflow, then a Release Manager (or anyone else) can easily see which tickets need to be reviewed before a release happens and which ones can be pushed out because they are not ready (even if a PR has been posted). It also makes it much easier for reviewers to quickly know which tickets are awaiting review. Thoughts? -Mark On Feb 23, 2017, at 3:37 AM, Andy LoPresto <alopresto.apa...@gmail.com<mailto:alopresto.apa...@gmail.com><mailto:alopresto.apa...@gmail.com>> wrote: As someone who has surely been guilty of optimistically setting fix versions and then not meeting them, I second Joe's point about it holding up releases. Better to get the PR out, reviewed, and merged *before* setting the fix version in my opinion. Andy LoPresto alopre...@apache.org<mailto:alopre...@apache.org><mailto:alopre...@apache.org> alopresto.apa...@gmail.com<mailto:alopresto.apa...@gmail.com><mailto:alopresto.apa...@gmail.com> PGP Fingerprint: 70EC B3E5 98A6 5A3F D3C4 BACE 3C6E F65B 2F7D EF69 On Feb 22, 2017, at 19:39, Joe Witt <joe.w...@gmail.com<mailto:joe.w...@gmail.com>> wrote: Peter, This is just my preference so discussion is certainly open. But the way I see it we should not set the fix version on JIRAs unless they really should block a release without resolution or if due to some roadmap/planning/discussion it is a new feature/improvement that is tied to a release. Otherwise, for the many things which pop up throughout a given release cycle they should be avoided. That is to say the majority of the time we'd avoid fix versions until the act of merging a contribution which also means it has been reviewed. From the release management point of view: This approach helps greatly as until now it is has been really difficult and time consuming to pull together/close down a release as pretty much anyone can set these fix versions and make it appear as though the release is not ready when in reality it is perfectly releasable as-is but might miss out on some contribs that someone would like to see in the release but has as of yet not gotten the PR and/or review traction necessary. From the contributor point of view: If someone makes a contribution they obviously want that code to end up in a release. But being an RTC community we need and want peer review before the code is submitted. Some contributions are frankly hard to get peer review on or simply take time for someone to volunteer to do. PRs which are difficult to test, lack testing, are related to systems or environments which are not easily replicated, etc.. are inherently harder to get peer review for. Also, the community has grown quite rapidly and sometimes the hygiene of a given PR isn't great. So our 'patch available' and 'open PR' count ticks up. We need reviews/feedback as much as we need contributions so it is important for folks that want those contributions in to build meritocracy as well in reviewing others contributions. This helps build a network of contributors/reviewers and improves the timeliness of reviews. Long story short here is that because at times PRs can sit too long sometimes people put a fix version on the JIRA so it acts as a sort of 'gating function' on the release. This I am saying is the practice that should not occur (given the thoughts above). We should instead take the issue of how to more effectively triage/review/provide feedback/and manage expectations for contributions so contributors don't feel like their stuff will just sit forever. Does that make sense and seem fair? Thanks Joe On Wed, Feb 22, 2017 at 2:39 PM, Peter Wicks (pwicks) <pwi...@micron.com<mailto:pwi...@micron.com>> wrote: Just for clarification, "We really need to avoid the practice of setting fix versions without traction", would mean don't set a version number until after we've submitted a PR? Until after the PR has been closed? Other? Thanks, Peter -----Original Message----- From: Joe Witt [mailto:joe.w...@gmail.com] Sent: Wednesday, February 22, 2017 12:55 PM To: dev@nifi.apache.org<mailto:dev@nifi.apache.org> Subject: Closing in on a NiFi 1.2.0 release? team, On the users lists we had an ask of when we are planning to cut a 1.2.0 release. And someone else asked me recently off-list. There are 45 open JIRAs tagged to it as of now. https://issues.apache.org/jira/issues/?jql=project%20%3D%20NIFI%20AND%20fixVersion%20%3D%201.2.0%20AND%20resolution%20%3D%20Unresolved%20ORDER%20BY%20priority%20DESC%2C%20key%20DESC I'd be favorable to going through and seeing if we can start the motions for a 1.2.0 release and which are ones we can wait for and which we should have in 1.2.0 for sure. Is there any reason folks can think of to hold off on a 1.2.0 release? A non trivial number of the JIRAs are for things which have or do not have PRs but have no review traction. We really need to avoid the practice of setting fix versions without traction on this as otherwise it holds up the releases. Thanks Joe
