On Mon, May 11, 2015 at 7:31 AM, Anthony Baker <[email protected]> wrote: > Roman, thanks for laying out these steps! Comments below…
NP! Like I said -- I'll put them on a wiki this week as well. Just so we can evolve *some* kind of a document. >> On May 9, 2015, at 1:29 PM, Roman Shaposhnik <[email protected]> wrote: >> >> Hi! >> >> I've just submitted GEODE-19 according to the >> process we seem to have agreed to in the DICUSS >> thread. Here I would like to take a moment and outline >> the steps I went through. If we like them I can document >> (and perhaps even automate) them. >> >> 1. I created the JIRA with the title and description taken >> from the original pull request >> >> 2. I linked the original pull request via More > Link JIRA >> functionality >> >> 3. I then downloaded the original patch correspondit >> to the pull request from: >> https://github.com/apache/incubator-geode/pull/1.patch >> and git am applied it to my repo. Note that this way all >> the original author and commit data is preserved. >> > > Seems like we should automate the above steps. +1 to that. Spark guys have a nice set of tools around this kind of automation. They are mostly written in Python and may not be easy to adopt but William told me he wanted to double click and see if there's any 'there' there. >> 4. I had to modify the patch slightly. Note that this step >> could be optional -- I could've provided that feedback >> to Mark and asked him to update the pull request accordingly. >> > > IMO it’s easier to discuss changes in context. Using either reviewboard or > GH for this is really helpful. Yup. As a pure code review tool both are fine. Like I said GH is a non-starter for archiving hence the requirement for patch actually being attached to a JIRA even if GH is used as a review tool. >> 7. I created a patch via git format-patch and attached it to JIRA >> asking for review. Once again -- attaching patches in git format-patch >> format is really important since it lets committers preserve >> the authorship information intact when comminting on other >> contributors behalf. > > In general I’d like to see a single view of the evolution of a patch. > Review board does this nicely as does a PR. Agreed. With projects like Hadoop what I've seen is that they always start with attaching patch to a JIRA and then if a reviewer asks for it the discussion moves to RB. Also, if we can invest in automation some of this steps will be as painless as running a script. > Once a reviewer gives a thumbs on the change are you suggesting > that the final patch get attached to the JIRA? Actually I was suggesting that the initial patch gets attached to JIRA. A lot of times it is small enough and then +1 can be given without any further questions or clarifications. If not -- reviewer will ask for RB or GH PR. > I’d rather see a commit hash to link the actual change to the ticket. That'd be awesome too. > The review comments would be archived in reviewboard or on the dev > list for GH right? Correct. But your HASH to JIRA linking approach only applies to patches that actually end up being committed (unless we're interested in intertaining the idea of a mob branch). >> 8. Once I get the required +1 I'm going to push to the master >> branch. That will close the PR #1 automagically (because my >> commit message has closes #1 test in it) but I will still have >> to close the JIRA manually once I see that fix made it. >> > > Is there auto-magik integration for JIRA as well? Such as described in > > https://confluence.atlassian.com/display/JIRACLOUD/Processing+JIRA+issues+with+commit+messages > > <https://confluence.atlassian.com/display/JIRACLOUD/Processing+JIRA+issues+with+commit+messages> I asked around and so far this doesn't seem to be easy on ASF INFRA. Here's a quote from somebody pretty close to the beating heart of INFRA: (a) I don't know if we want state transitions from commit messages and (b) that requires the JIRA DVCS connector, which we most definitely don't have installed, and which (all told, given some pain I know of people having with it) I don't think we want installed. I can nag some more, but at this point I'd rather invest in a script (or set of Gradle tasks) that automate all of it on the client end(*) Thanks, Roman. (*) which is easy for me to say since I don't plan to write that code anytime soon ;-)
