This is good discussion that should continue, but what about the original intent of Joe's post? "Is there any reason folks can think of to hold off on a 1.2.0 release?"
On Fri, Mar 3, 2017 at 1:45 PM, Mark Payne <marka...@hotmail.com> wrote: > 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:a > lopre...@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:m > arka...@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:a > lopre...@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:alo > pre...@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:m > arka...@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:alo > pre...@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 > > -- I know what it is to be in need, and I know what it is to have plenty. I have learned the secret of being content in any and every situation, whether well fed or hungry, whether living in plenty or in want. I can do all this through him who gives me strength. *-Philippians 4:12-13*
