You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@geode.apache.org by Roman Shaposhnik <ro...@shaposhnik.org> on 2015/05/09 22:29:13 UTC

A few notes about GEODE-19

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.

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.

5. Once I was satisfied with the result I squashed all
the commits into a single one corresponding to a JIRA.
I am going to strongly advocate one JIRA -- one commit
rule in most of the cases.

6. I merged all the comments from all the commits, but also
made sure that the first line of the commit message follows
the standard of: JIRA-ID. Jira tittle (closes #PR)
    GEODE-19. Support for creating Maven distribution (closes #1)
This is very important to be able to build CHANGES.txt
file and also close the PRs automatically. The rest of the commit
message is free form.

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.

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.

Thanks,
Roman.

Re: A few notes about GEODE-19

Posted by Justin Erenkrantz <ju...@erenkrantz.com>.
On Sat, May 9, 2015 at 4:29 PM, Roman Shaposhnik <ro...@shaposhnik.org> wrote:
> 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

A suggestion to consider: if we are going to a very JIRA-centric
development workflow, perhaps we should re-target the JIRA
notifications to dev@ rather than a separate issues@ list at this
time?  -- justin

Re: A few notes about GEODE-19

Posted by Dan Smith <ds...@pivotal.io>.
On Mon, May 11, 2015 at 7:31 AM, Anthony Baker <ab...@pivotal.io> wrote:

> Roman, thanks for laying out these steps!  Comments below…
>
> Once a reviewer gives a thumbs on the change are you suggesting that the
> final patch get attached to the JIRA?  I’d rather see a commit hash to link
> the actual change to the ticket.  The review comments would be archived in
> reviewboard or on the dev list for GH right?
>
> >
> > 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
> >
>

+1 for updating JIRA from the commit message. I think a JIRA should link to
the commit that fixed the JIRA. It would be nice if that happened
automatically. Atlassian looks like they have an integration with github,
but we probably want to make sure the JIRA links back to the ASF gitweb, so
we might have to write our own hook.

-Dan

Re: A few notes about GEODE-19

Posted by Dan Smith <ds...@pivotal.io>.
I do see something that looks like maybe it's supported by ASF INFRA to add
a comment to the JIRA:

http://www.apache.org/dev/svngit2jira.html

That won't close the ticket, but maybe a comment is good enough? Looks like
it also has some functionality to auto-link to reviewboard as well. I'd be
in favor of hooking this up if we can.

-Dan

On Tue, May 12, 2015 at 3:32 PM, Roman Shaposhnik <rv...@apache.org> wrote:

> On Mon, May 11, 2015 at 7:31 AM, Anthony Baker <ab...@pivotal.io> 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 <ro...@shaposhnik.org>
> 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 ;-)
>

Re: A few notes about GEODE-19

Posted by Roman Shaposhnik <rv...@apache.org>.
On Mon, May 11, 2015 at 7:31 AM, Anthony Baker <ab...@pivotal.io> 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 <ro...@shaposhnik.org> 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 ;-)

Re: A few notes about GEODE-19

Posted by Anthony Baker <ab...@pivotal.io>.
Roman, thanks for laying out these steps!  Comments below…

> On May 9, 2015, at 1:29 PM, Roman Shaposhnik <ro...@shaposhnik.org> 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.

> 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.

> 5. Once I was satisfied with the result I squashed all
> the commits into a single one corresponding to a JIRA.
> I am going to strongly advocate one JIRA -- one commit
> rule in most of the cases.
> 

This makes sense in most cases.  (Let’s avoid bringing feature branches into the discussion for now)

> 6. I merged all the comments from all the commits, but also
> made sure that the first line of the commit message follows
> the standard of: JIRA-ID. Jira tittle (closes #PR)
>    GEODE-19. Support for creating Maven distribution (closes #1)
> This is very important to be able to build CHANGES.txt
> file and also close the PRs automatically. The rest of the commit
> message is free form.
> 

Yes.  Read more about git commit messages practices in posts like this:
http://chris.beams.io/posts/git-commit/ <http://chris.beams.io/posts/git-commit/>


> 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.  

Once a reviewer gives a thumbs on the change are you suggesting that the final patch get attached to the JIRA?  I’d rather see a commit hash to link the actual change to the ticket.  The review comments would be archived in reviewboard or on the dev list for GH right?
 
> 
> 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>


> Thanks,
> Roman.

Anthony