You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cloudstack.apache.org by sebgoa <ru...@gmail.com> on 2012/07/31 22:24:00 UTC
non-committer workflow
Hi,
Since I am new to Cloudstack and Apache like projects, I was looking at the process for submitting a very small patch (e.g typo, one-liner, license header)
The wiki page on how to use git o contribute to Cloudstack has a section for non-committer:
http://wiki.cloudstack.org/display/dev/Git+workflow+in+the+brave+new+world#Gitworkflowinthebravenewworld-Non-committerworkflow
This is a bit confusing to me, so I would like to get some feedback from the more seasoned developper and committers:
1-Does every patch needs to be submitted in response to a filed bug ?
2-Do we got through https://reviews.apache.org ?
3-The wiki mentions sending the patch to dev mailing list with subject [PATCH] but that's not what I have seen in the last few weeks ?
4-What's the best way to use git to prepare a small patch (e.g docs typo, one-liner ...etc), do we really need to create a branch ? how does it get resolved locally once the patch is accepted ?
It would be nice to agree on a clear worfklow for non-committers (and newbies like me) so we can get some people in the mix and graduate to committers...
Thoughts anyone ?
Thanks,
-Sebastien Goasguen
Apache Cloudstack Evangelist
https://sites.google.com/site/runseb/
RE: non-committer workflow
Posted by Edison Su <Ed...@citrix.com>.
> -----Original Message-----
> From: David Nalley [mailto:david@gnsa.us]
> Sent: Tuesday, July 31, 2012 2:14 PM
> To: cloudstack-dev@incubator.apache.org
> Subject: Re: non-committer workflow
>
> On Tue, Jul 31, 2012 at 5:05 PM, sebgoa <ru...@gmail.com> wrote:
> >
> > So Non-committers don't need to get an account on RB and submit
> patches via that path ?
> >
>
> So currently, there are three ways for a patch to be received:
> 1. Email (see the workflow Wido proposed)
> 2. Reviewboard
> 3. Submitted with a bug.
>
> Email and ReviewBoard are the most visible, and it seems most people
> are using ReviewBoard rather than email.
Use ReviewBoard as much as possible!!
>
> --David
Re: non-committer workflow
Posted by Chip Childers <ch...@sungard.com>.
On Tue, Jul 31, 2012 at 10:50 PM, Alex Huang <Al...@citrix.com> wrote:
>> So currently, there are three ways for a patch to be received:
>> 1. Email (see the workflow Wido proposed) 2. Reviewboard 3. Submitted
>> with a bug.
>>
>> Email and ReviewBoard are the most visible, and it seems most people are
>> using ReviewBoard rather than email.
>
> We should remove the email and submit with a bug options.
>
> --Alex
>
+1 - While the other options help establish provenance for the code,
ReviewBoard makes it easier to track / comment. I also agree with
Ewan that we should have more tooling around applying patches.
-chip
Re: non-committer workflow
Posted by Rohit Yadav <ro...@citrix.com>.
Hi Prasanna,
On Aug 1, 2012, at 6:26 PM, Prasanna Santhanam <Pr...@citrix.com> wrote:
> On Wed, Aug 01, 2012 at 08:42:51AM -0400, Rohit Yadav wrote:
>>
>> On Aug 1, 2012, at 5:46 PM, Wido den Hollander <wi...@widodh.nl> wrote:
>>
>>> On 08/01/2012 04:50 AM, Alex Huang wrote:
>>>>> So currently, there are three ways for a patch to be received:
>>>>> 1. Email (see the workflow Wido proposed) 2. Reviewboard 3. Submitted
>>>>> with a bug.
>>>>>
>>>>> Email and ReviewBoard are the most visible, and it seems most people are
>>>>> using ReviewBoard rather than email.
>>>>
>>>> We should remove the email and submit with a bug options.
>>>>
>>>
>>> Are you sure? For larger patches I agree that e-mail isn't that easy,
>>> but it seems to work with various other projects.
>>>
>>> I personally like e-mail and 'hate' all kinds of various systems where I
>>> have to log in with different credentials.
>>
>> I totally agree with Wido, besides the review board workflow has a serious ownership issue:
>>
>> When I submit a git formatted patch it would strip the commit
>> message and author info, signature and retain only the unified diff.
>> The reviewer and/or committer have to do the extra work of
>> downloading the patch, applying/verifying it and then committing it.
>> During the process they may change the original commit message and
>> author signature (and they lose their ohloh points :)
>>
>> Further, the contributor is then required to go back and close the
>> submission. Well one can use their mailbox from where they can
>> import the patches, git am works. Or when the committer is
>> downloading the diff anyway, why not download the actual git
>> formatted patch emailed by an author and git apply? Just a
>> suggestion.
>>
>
> This was tried in the past and backfired when non-committers send
> through patches that get formatted by mail clients and have CRLF
> issues when applied by the committer.
>
> I agree Reviewboard has extra workflow involved but it integrates the
> review comments closely with the mailing list so it isn't as different
> from patches in the email in my opinion. It additionally provides
> pretty diff tools online. If not I would have to put the patch through
> my own diff tool.
>
> The current pain points as I see are :
>
> 1) ReviewBoard removes the author information from the diff
> 2) git am doesn't always work
> 3) extra workflow step of submitter closing the patch request
>
> These probably should be addressed by tooling.
I understand. So, I've opened a ticket here with the review board's bug tracker:
http://code.google.com/p/reviewboard/issues/detail?id=2690&thanks=2690&ts=1343826104
One accepted solution can be that we see the submitted patch in unified diff format on the review board, but when you download it, you get the original file which was uploaded by the patch author. If you agree, please vote/comment on that ticket and please forward our request to Apache as well.
Thanks,
Rohit
>
>>
>>>
>>> If somebody finds a typo or small bug, should they really go through all
>>> the hoops for submitting a small patch?
>>>
>>> Imho that shouldn't be necessary.
>>>
>>> Wido
>
> --
> Prasanna.,
Re: non-committer workflow
Posted by Rohit Yadav <ro...@citrix.com>.
Hi Pradeep,
On Aug 2, 2012, at 11:29 AM, Pradeep Soundararajan <pr...@citrix.com> wrote:
> Hi Rohit,
>
> I have already created the RB wiki for our team. Could you please post your findings there if anything is not covered.
>
> http://wiki.cloudstack.org/display/gen/Review+Board
>
> Also, please let me know are you able to post your review through command line?
Yes, it works for me. The problem is with the RB and not the command line tool, I've shared a workaround that I'll be using in future till RB guys fix the issue.
Rohit
>
> The last issue I have found before I have come out from RB is most of them are unable to post their review through command line. I am able to post the same after some minor tweak. It will be good if you elaborate this….
>
> Thanks,
> Pradeep.S
>
>
> -----Original Message-----
> From: Rohit Yadav [mailto:rohit.yadav@citrix.com]
> Sent: Thursday, August 02, 2012 2:13 AM
> To: cloudstack-dev@incubator.apache.org
> Subject: RE: non-committer workflow
>
> Hi Rajesh,
> ________________________________________
> From: Rajesh Battala [rajesh.battala@citrix.com]
> Sent: Wednesday, August 01, 2012 8:46 PM
> To: cloudstack-dev@incubator.apache.org
> Subject: RE: non-committer workflow
>
> Rohit,
>
> Pradeep had played and tweaked the same tools for our team. If you are going to share about the tool, you can talk to Pradeep to share more stuff about it .
>
> Sure, I almost missed your email and went ahead with a tweak. Anyway, it works for me.
>
> Regards.
>
> Thanks
> Rajesh Battala
>
>
> -----Original Message-----
> From: Rohit Yadav [mailto:rohit.yadav@citrix.com]
> Sent: Wednesday, August 01, 2012 8:03 PM
> To: cloudstack-dev@incubator.apache.org
> Subject: Re: non-committer workflow
>
> Hi,
>
> Prasanna and I've been playing with the http://downloads.reviewboard.org/releases/RBTools/0.4/RBTools-0.4.1.tar.gz tool for posting the reviews via a command line utility.
>
> We can tweak the script easily so when you submit a review request, the original git formatted patch is uploaded to some public hosting site and will append the link in the description. The committer can then get the original patch with all author's info and apply it using git am.
>
> Regards,
> Rohit
>
> On Aug 1, 2012, at 6:54 PM, Prasanna Santhanam <Pr...@citrix.com> wrote:
>
>> On Wed, Aug 01, 2012 at 09:07:07AM -0400, Wido den Hollander wrote:
>>>>
>>>> This was tried in the past and backfired when non-committers send
>>>> through patches that get formatted by mail clients and have CRLF
>>>> issues when applied by the committer.
>>>>
>>>
>>> I think this happens when people attach their patches, but if you
>>> send them with "git send-email" they will go through just fine.
>>>
>>> HTML mail clients and stuff make garbage of patches. That's why I'm
>>> again HTML e-mail on this mailinglist.
>>>
>>
>> True - it's not necessarily the non-committer sending it through an
>> HTML client but some of our committers are forced in one way or
>> another to adhere to Outlook like clients.
>>
>>
>>>> 3) extra workflow step of submitter closing the patch request
>>>>
>>>> These probably should be addressed by tooling.
>>>
>>> Do you mean reviewboard tooling or tooling for patches through e-mail?
>>>
>>
>> I meant reviewboard tooling/fix so it doesn't strip out author
>> information and so that git am works. Rohit's beaten me to the request
>> with RB's team. It might take too much time before apache infra
>> decides to upgrade the reviews.a.o though.
>>
>> --
>> Prasanna.,
>
RE: non-committer workflow
Posted by Pradeep Soundararajan <pr...@citrix.com>.
Hi Rohit,
I have already created the RB wiki for our team. Could you please post your findings there if anything is not covered.
http://wiki.cloudstack.org/display/gen/Review+Board
Also, please let me know are you able to post your review through command line?
The last issue I have found before I have come out from RB is most of them are unable to post their review through command line. I am able to post the same after some minor tweak. It will be good if you elaborate this....
Thanks,
Pradeep.S
-----Original Message-----
From: Rohit Yadav [mailto:rohit.yadav@citrix.com]
Sent: Thursday, August 02, 2012 2:13 AM
To: cloudstack-dev@incubator.apache.org
Subject: RE: non-committer workflow
Hi Rajesh,
________________________________________
From: Rajesh Battala [rajesh.battala@citrix.com]
Sent: Wednesday, August 01, 2012 8:46 PM
To: cloudstack-dev@incubator.apache.org
Subject: RE: non-committer workflow
Rohit,
Pradeep had played and tweaked the same tools for our team. If you are going to share about the tool, you can talk to Pradeep to share more stuff about it .
Sure, I almost missed your email and went ahead with a tweak. Anyway, it works for me.
Regards.
Thanks
Rajesh Battala
-----Original Message-----
From: Rohit Yadav [mailto:rohit.yadav@citrix.com]
Sent: Wednesday, August 01, 2012 8:03 PM
To: cloudstack-dev@incubator.apache.org
Subject: Re: non-committer workflow
Hi,
Prasanna and I've been playing with the http://downloads.reviewboard.org/releases/RBTools/0.4/RBTools-0.4.1.tar.gz tool for posting the reviews via a command line utility.
We can tweak the script easily so when you submit a review request, the original git formatted patch is uploaded to some public hosting site and will append the link in the description. The committer can then get the original patch with all author's info and apply it using git am.
Regards,
Rohit
On Aug 1, 2012, at 6:54 PM, Prasanna Santhanam <Pr...@citrix.com> wrote:
> On Wed, Aug 01, 2012 at 09:07:07AM -0400, Wido den Hollander wrote:
>>>
>>> This was tried in the past and backfired when non-committers send
>>> through patches that get formatted by mail clients and have CRLF
>>> issues when applied by the committer.
>>>
>>
>> I think this happens when people attach their patches, but if you
>> send them with "git send-email" they will go through just fine.
>>
>> HTML mail clients and stuff make garbage of patches. That's why I'm
>> again HTML e-mail on this mailinglist.
>>
>
> True - it's not necessarily the non-committer sending it through an
> HTML client but some of our committers are forced in one way or
> another to adhere to Outlook like clients.
>
>
>>> 3) extra workflow step of submitter closing the patch request
>>>
>>> These probably should be addressed by tooling.
>>
>> Do you mean reviewboard tooling or tooling for patches through e-mail?
>>
>
> I meant reviewboard tooling/fix so it doesn't strip out author
> information and so that git am works. Rohit's beaten me to the request
> with RB's team. It might take too much time before apache infra
> decides to upgrade the reviews.a.o though.
>
> --
> Prasanna.,
RE: non-committer workflow
Posted by Rohit Yadav <ro...@citrix.com>.
Hi Rajesh,
________________________________________
From: Rajesh Battala [rajesh.battala@citrix.com]
Sent: Wednesday, August 01, 2012 8:46 PM
To: cloudstack-dev@incubator.apache.org
Subject: RE: non-committer workflow
Rohit,
Pradeep had played and tweaked the same tools for our team. If you are going to share about the tool, you can talk to Pradeep to share more stuff about it .
Sure, I almost missed your email and went ahead with a tweak. Anyway, it works for me.
Regards.
Thanks
Rajesh Battala
-----Original Message-----
From: Rohit Yadav [mailto:rohit.yadav@citrix.com]
Sent: Wednesday, August 01, 2012 8:03 PM
To: cloudstack-dev@incubator.apache.org
Subject: Re: non-committer workflow
Hi,
Prasanna and I've been playing with the http://downloads.reviewboard.org/releases/RBTools/0.4/RBTools-0.4.1.tar.gz tool for posting the reviews via a command line utility.
We can tweak the script easily so when you submit a review request, the original git formatted patch is uploaded to some public hosting site and will append the link in the description. The committer can then get the original patch with all author's info and apply it using git am.
Regards,
Rohit
On Aug 1, 2012, at 6:54 PM, Prasanna Santhanam <Pr...@citrix.com> wrote:
> On Wed, Aug 01, 2012 at 09:07:07AM -0400, Wido den Hollander wrote:
>>>
>>> This was tried in the past and backfired when non-committers send
>>> through patches that get formatted by mail clients and have CRLF
>>> issues when applied by the committer.
>>>
>>
>> I think this happens when people attach their patches, but if you
>> send them with "git send-email" they will go through just fine.
>>
>> HTML mail clients and stuff make garbage of patches. That's why I'm
>> again HTML e-mail on this mailinglist.
>>
>
> True - it's not necessarily the non-committer sending it through an
> HTML client but some of our committers are forced in one way or
> another to adhere to Outlook like clients.
>
>
>>> 3) extra workflow step of submitter closing the patch request
>>>
>>> These probably should be addressed by tooling.
>>
>> Do you mean reviewboard tooling or tooling for patches through e-mail?
>>
>
> I meant reviewboard tooling/fix so it doesn't strip out author
> information and so that git am works. Rohit's beaten me to the request
> with RB's team. It might take too much time before apache infra
> decides to upgrade the reviews.a.o though.
>
> --
> Prasanna.,
RE: non-committer workflow
Posted by Rajesh Battala <ra...@citrix.com>.
Rohit,
Pradeep had played and tweaked the same tools for our team. If you are going to share about the tool, you can talk to Pradeep to share more stuff about it .
Thanks
Rajesh Battala
-----Original Message-----
From: Rohit Yadav [mailto:rohit.yadav@citrix.com]
Sent: Wednesday, August 01, 2012 8:03 PM
To: cloudstack-dev@incubator.apache.org
Subject: Re: non-committer workflow
Hi,
Prasanna and I've been playing with the http://downloads.reviewboard.org/releases/RBTools/0.4/RBTools-0.4.1.tar.gz tool for posting the reviews via a command line utility.
We can tweak the script easily so when you submit a review request, the original git formatted patch is uploaded to some public hosting site and will append the link in the description. The committer can then get the original patch with all author's info and apply it using git am.
Regards,
Rohit
On Aug 1, 2012, at 6:54 PM, Prasanna Santhanam <Pr...@citrix.com> wrote:
> On Wed, Aug 01, 2012 at 09:07:07AM -0400, Wido den Hollander wrote:
>>>
>>> This was tried in the past and backfired when non-committers send
>>> through patches that get formatted by mail clients and have CRLF
>>> issues when applied by the committer.
>>>
>>
>> I think this happens when people attach their patches, but if you
>> send them with "git send-email" they will go through just fine.
>>
>> HTML mail clients and stuff make garbage of patches. That's why I'm
>> again HTML e-mail on this mailinglist.
>>
>
> True - it's not necessarily the non-committer sending it through an
> HTML client but some of our committers are forced in one way or
> another to adhere to Outlook like clients.
>
>
>>> 3) extra workflow step of submitter closing the patch request
>>>
>>> These probably should be addressed by tooling.
>>
>> Do you mean reviewboard tooling or tooling for patches through e-mail?
>>
>
> I meant reviewboard tooling/fix so it doesn't strip out author
> information and so that git am works. Rohit's beaten me to the request
> with RB's team. It might take too much time before apache infra
> decides to upgrade the reviews.a.o though.
>
> --
> Prasanna.,
Re: non-committer workflow
Posted by prasanna <sr...@gmail.com>.
On 2 August 2012 04:10, Edison Su <Ed...@citrix.com> wrote:
>
>
>> -----Original Message-----
>> From: Rohit Yadav [mailto:rohit.yadav@citrix.com]
>> Sent: Wednesday, August 01, 2012 1:29 PM
>> To: cloudstack-dev@incubator.apache.org
>> Subject: Re: non-committer workflow
>>
>>
>> > Hi,
>> >
>> > Prasanna and I've been playing with the
>> http://downloads.reviewboard.org/releases/RBTools/0.4/RBTools-
>> 0.4.1.tar.gz tool for posting the reviews via a command line utility.
>>
>> I've a fork of the original tool that works for me:
>> https://github.com/bhaisaab/RBTool
>>
>> Usage: http://wiki.cloudstack.org/display/gen/Review+Board
>>
>> Example:
>> git format-patch -o patches HEAD~1
>> postreview --username=<user-name> --password=<password> --diff-
>> filename=patches/0001-myfix.patch --debug --description="description-
>> of-my-patch" #(add -p if you want to publish right away)
>> Test: https://reviews.apache.org/r/6299/
>>
>> The patch is uploaded on paste.cloudstack.org and the link is appended
>> to the description.
>
> Can we upload the patch to somewhere? The link is an url, you can't directly download it(for people on windows, there is no wget)
>
Neat hack for the time being. On a browser you can simply do "Save
Target As./Save Link As". It works well and I can see the author
information in there.
--
Prasanna.,
RE: non-committer workflow
Posted by Edison Su <Ed...@citrix.com>.
> -----Original Message-----
> From: Rohit Yadav [mailto:rohit.yadav@citrix.com]
> Sent: Thursday, August 02, 2012 12:14 AM
> To: cloudstack-dev@incubator.apache.org
> Subject: Re: non-committer workflow
>
>
> On Aug 2, 2012, at 4:10 AM, Edison Su <Ed...@citrix.com> wrote:
> >>
> >> The patch is uploaded on paste.cloudstack.org and the link is
> appended
> >> to the description.
> >
> > Can we upload the patch to somewhere? The link is an url, you can't
> directly download it(for people on windows, there is no wget)
>
> Yes, that is easily do-able. If you want to use your own service or
> server, fork my repo:
> https://github.com/bhaisaab/RBTool/commit/1b7c4742179986b016087044436a2
> 1fbc9700a48
Great tool, it will simplify the process for both committer and non-committer.
>
> Okay, I've modified the tool to upload the patch to a patch bin
> (https://github.com/bhaisaab/RBTool/commit/caa89849b75cf2b20331ad78daad
> df9d688320de) I created on http://patchbin.baagi.org which force
> downloads on your browser.
>
> Rohit
Re: non-committer workflow
Posted by Rohit Yadav <ro...@citrix.com>.
On Aug 2, 2012, at 4:10 AM, Edison Su <Ed...@citrix.com> wrote:
>>
>> The patch is uploaded on paste.cloudstack.org and the link is appended
>> to the description.
>
> Can we upload the patch to somewhere? The link is an url, you can't directly download it(for people on windows, there is no wget)
Yes, that is easily do-able. If you want to use your own service or server, fork my repo:
https://github.com/bhaisaab/RBTool/commit/1b7c4742179986b016087044436a21fbc9700a48
Okay, I've modified the tool to upload the patch to a patch bin (https://github.com/bhaisaab/RBTool/commit/caa89849b75cf2b20331ad78daaddf9d688320de) I created on http://patchbin.baagi.org which force downloads on your browser.
Rohit
RE: non-committer workflow
Posted by Edison Su <Ed...@citrix.com>.
> -----Original Message-----
> From: Rohit Yadav [mailto:rohit.yadav@citrix.com]
> Sent: Wednesday, August 01, 2012 1:29 PM
> To: cloudstack-dev@incubator.apache.org
> Subject: Re: non-committer workflow
>
>
> > Hi,
> >
> > Prasanna and I've been playing with the
> http://downloads.reviewboard.org/releases/RBTools/0.4/RBTools-
> 0.4.1.tar.gz tool for posting the reviews via a command line utility.
>
> I've a fork of the original tool that works for me:
> https://github.com/bhaisaab/RBTool
>
> Usage: http://wiki.cloudstack.org/display/gen/Review+Board
>
> Example:
> git format-patch -o patches HEAD~1
> postreview --username=<user-name> --password=<password> --diff-
> filename=patches/0001-myfix.patch --debug --description="description-
> of-my-patch" #(add -p if you want to publish right away)
> Test: https://reviews.apache.org/r/6299/
>
> The patch is uploaded on paste.cloudstack.org and the link is appended
> to the description.
Can we upload the patch to somewhere? The link is an url, you can't directly download it(for people on windows, there is no wget)
>
> Hope it works.
>
> Regards,
> Rohit
>
> >
> > We can tweak the script easily so when you submit a review request,
> the original git formatted patch is uploaded to some public hosting
> site and will append the link in the description. The committer can
> then get the original patch with all author's info and apply it using
> git am.
> >
> > Regards,
> > Rohit
> >
> > On Aug 1, 2012, at 6:54 PM, Prasanna Santhanam
> <Pr...@citrix.com> wrote:
> >
> >> On Wed, Aug 01, 2012 at 09:07:07AM -0400, Wido den Hollander wrote:
> >>>>
> >>>> This was tried in the past and backfired when non-committers send
> >>>> through patches that get formatted by mail clients and have CRLF
> >>>> issues when applied by the committer.
> >>>>
> >>>
> >>> I think this happens when people attach their patches, but if you
> send
> >>> them with "git send-email" they will go through just fine.
> >>>
> >>> HTML mail clients and stuff make garbage of patches. That's why I'm
> >>> again HTML e-mail on this mailinglist.
> >>>
> >>
> >> True - it's not necessarily the non-committer sending it through an
> >> HTML client but some of our committers are forced in one way or
> >> another to adhere to Outlook like clients.
> >>
> >>
> >>>> 3) extra workflow step of submitter closing the patch request
> >>>>
> >>>> These probably should be addressed by tooling.
> >>>
> >>> Do you mean reviewboard tooling or tooling for patches through e-
> mail?
> >>>
> >>
> >> I meant reviewboard tooling/fix so it doesn't strip out author
> >> information and so that git am works. Rohit's beaten me to the
> request
> >> with RB's team. It might take too much time before apache infra
> >> decides to upgrade the reviews.a.o though.
> >>
> >> --
> >> Prasanna.,
> >
Re: non-committer workflow
Posted by Rohit Yadav <ro...@citrix.com>.
> Hi,
>
> Prasanna and I've been playing with the http://downloads.reviewboard.org/releases/RBTools/0.4/RBTools-0.4.1.tar.gz tool for posting the reviews via a command line utility.
I've a fork of the original tool that works for me:
https://github.com/bhaisaab/RBTool
Usage: http://wiki.cloudstack.org/display/gen/Review+Board
Example:
git format-patch -o patches HEAD~1
postreview --username=<user-name> --password=<password> --diff-filename=patches/0001-myfix.patch --debug --description="description-of-my-patch" #(add -p if you want to publish right away)
Test: https://reviews.apache.org/r/6299/
The patch is uploaded on paste.cloudstack.org and the link is appended to the description.
Hope it works.
Regards,
Rohit
>
> We can tweak the script easily so when you submit a review request, the original git formatted patch is uploaded to some public hosting site and will append the link in the description. The committer can then get the original patch with all author's info and apply it using git am.
>
> Regards,
> Rohit
>
> On Aug 1, 2012, at 6:54 PM, Prasanna Santhanam <Pr...@citrix.com> wrote:
>
>> On Wed, Aug 01, 2012 at 09:07:07AM -0400, Wido den Hollander wrote:
>>>>
>>>> This was tried in the past and backfired when non-committers send
>>>> through patches that get formatted by mail clients and have CRLF
>>>> issues when applied by the committer.
>>>>
>>>
>>> I think this happens when people attach their patches, but if you send
>>> them with "git send-email" they will go through just fine.
>>>
>>> HTML mail clients and stuff make garbage of patches. That's why I'm
>>> again HTML e-mail on this mailinglist.
>>>
>>
>> True - it's not necessarily the non-committer sending it through an
>> HTML client but some of our committers are forced in one way or
>> another to adhere to Outlook like clients.
>>
>>
>>>> 3) extra workflow step of submitter closing the patch request
>>>>
>>>> These probably should be addressed by tooling.
>>>
>>> Do you mean reviewboard tooling or tooling for patches through e-mail?
>>>
>>
>> I meant reviewboard tooling/fix so it doesn't strip out author
>> information and so that git am works. Rohit's beaten me to the request
>> with RB's team. It might take too much time before apache infra
>> decides to upgrade the reviews.a.o though.
>>
>> --
>> Prasanna.,
>
Re: non-committer workflow
Posted by Rohit Yadav <ro...@citrix.com>.
Hi,
Prasanna and I've been playing with the http://downloads.reviewboard.org/releases/RBTools/0.4/RBTools-0.4.1.tar.gz tool for posting the reviews via a command line utility.
We can tweak the script easily so when you submit a review request, the original git formatted patch is uploaded to some public hosting site and will append the link in the description. The committer can then get the original patch with all author's info and apply it using git am.
Regards,
Rohit
On Aug 1, 2012, at 6:54 PM, Prasanna Santhanam <Pr...@citrix.com> wrote:
> On Wed, Aug 01, 2012 at 09:07:07AM -0400, Wido den Hollander wrote:
>>>
>>> This was tried in the past and backfired when non-committers send
>>> through patches that get formatted by mail clients and have CRLF
>>> issues when applied by the committer.
>>>
>>
>> I think this happens when people attach their patches, but if you send
>> them with "git send-email" they will go through just fine.
>>
>> HTML mail clients and stuff make garbage of patches. That's why I'm
>> again HTML e-mail on this mailinglist.
>>
>
> True - it's not necessarily the non-committer sending it through an
> HTML client but some of our committers are forced in one way or
> another to adhere to Outlook like clients.
>
>
>>> 3) extra workflow step of submitter closing the patch request
>>>
>>> These probably should be addressed by tooling.
>>
>> Do you mean reviewboard tooling or tooling for patches through e-mail?
>>
>
> I meant reviewboard tooling/fix so it doesn't strip out author
> information and so that git am works. Rohit's beaten me to the request
> with RB's team. It might take too much time before apache infra
> decides to upgrade the reviews.a.o though.
>
> --
> Prasanna.,
Re: non-committer workflow
Posted by Prasanna Santhanam <Pr...@citrix.com>.
On Wed, Aug 01, 2012 at 09:07:07AM -0400, Wido den Hollander wrote:
> >
> > This was tried in the past and backfired when non-committers send
> > through patches that get formatted by mail clients and have CRLF
> > issues when applied by the committer.
> >
>
> I think this happens when people attach their patches, but if you send
> them with "git send-email" they will go through just fine.
>
> HTML mail clients and stuff make garbage of patches. That's why I'm
> again HTML e-mail on this mailinglist.
>
True - it's not necessarily the non-committer sending it through an
HTML client but some of our committers are forced in one way or
another to adhere to Outlook like clients.
> > 3) extra workflow step of submitter closing the patch request
> >
> > These probably should be addressed by tooling.
>
> Do you mean reviewboard tooling or tooling for patches through e-mail?
>
I meant reviewboard tooling/fix so it doesn't strip out author
information and so that git am works. Rohit's beaten me to the request
with RB's team. It might take too much time before apache infra
decides to upgrade the reviews.a.o though.
--
Prasanna.,
Re: non-committer workflow
Posted by Wido den Hollander <wi...@widodh.nl>.
On 08/01/2012 02:56 PM, Prasanna Santhanam wrote:
> On Wed, Aug 01, 2012 at 08:42:51AM -0400, Rohit Yadav wrote:
>>
>> On Aug 1, 2012, at 5:46 PM, Wido den Hollander <wi...@widodh.nl> wrote:
>>
>>> On 08/01/2012 04:50 AM, Alex Huang wrote:
>>>>> So currently, there are three ways for a patch to be received:
>>>>> 1. Email (see the workflow Wido proposed) 2. Reviewboard 3. Submitted
>>>>> with a bug.
>>>>>
>>>>> Email and ReviewBoard are the most visible, and it seems most people are
>>>>> using ReviewBoard rather than email.
>>>>
>>>> We should remove the email and submit with a bug options.
>>>>
>>>
>>> Are you sure? For larger patches I agree that e-mail isn't that easy,
>>> but it seems to work with various other projects.
>>>
>>> I personally like e-mail and 'hate' all kinds of various systems where I
>>> have to log in with different credentials.
>>
>> I totally agree with Wido, besides the review board workflow has a serious ownership issue:
>>
>> When I submit a git formatted patch it would strip the commit
>> message and author info, signature and retain only the unified diff.
>> The reviewer and/or committer have to do the extra work of
>> downloading the patch, applying/verifying it and then committing it.
>> During the process they may change the original commit message and
>> author signature (and they lose their ohloh points :)
>>
>> Further, the contributor is then required to go back and close the
>> submission. Well one can use their mailbox from where they can
>> import the patches, git am works. Or when the committer is
>> downloading the diff anyway, why not download the actual git
>> formatted patch emailed by an author and git apply? Just a
>> suggestion.
>>
>
> This was tried in the past and backfired when non-committers send
> through patches that get formatted by mail clients and have CRLF
> issues when applied by the committer.
>
I think this happens when people attach their patches, but if you send
them with "git send-email" they will go through just fine.
HTML mail clients and stuff make garbage of patches. That's why I'm
again HTML e-mail on this mailinglist.
> I agree Reviewboard has extra workflow involved but it integrates the
> review comments closely with the mailing list so it isn't as different
> from patches in the email in my opinion. It additionally provides
> pretty diff tools online. If not I would have to put the patch through
> my own diff tool.
>
> The current pain points as I see are :
>
> 1) ReviewBoard removes the author information from the diff
> 2) git am doesn't always work
Again, I think because people did not use "git send-email"
> 3) extra workflow step of submitter closing the patch request
>
> These probably should be addressed by tooling.
Do you mean reviewboard tooling or tooling for patches through e-mail?
Wido
>
>>
>>>
>>> If somebody finds a typo or small bug, should they really go through all
>>> the hoops for submitting a small patch?
>>>
>>> Imho that shouldn't be necessary.
>>>
>>> Wido
>
Re: non-committer workflow
Posted by David Nalley <da...@gnsa.us>.
>
> I'm still very new to the project, but I found it very difficult to
> follow up what's going on because patches are in separate places.
> I'd argue there should be one single workflow so that we can better
> keep track of the status.
This is interesting feedback.
I was hoping we'd keep the barrier low by allowing patches to come in
via multiple means, but you seem to be saying that is more confusing.
Thanks for voicing this,
--David
Re: non-committer workflow
Posted by Tomoe Sugihara <to...@midokura.com>.
On Wed, Aug 1, 2012 at 9:56 PM, Prasanna Santhanam
<Pr...@citrix.com> wrote:
> On Wed, Aug 01, 2012 at 08:42:51AM -0400, Rohit Yadav wrote:
>>
>> On Aug 1, 2012, at 5:46 PM, Wido den Hollander <wi...@widodh.nl> wrote:
>>
>> > On 08/01/2012 04:50 AM, Alex Huang wrote:
>> >>> So currently, there are three ways for a patch to be received:
>> >>> 1. Email (see the workflow Wido proposed) 2. Reviewboard 3. Submitted
>> >>> with a bug.
>> >>>
>> >>> Email and ReviewBoard are the most visible, and it seems most people are
>> >>> using ReviewBoard rather than email.
>> >>
>> >> We should remove the email and submit with a bug options.
>> >>
>> >
>> > Are you sure? For larger patches I agree that e-mail isn't that easy,
>> > but it seems to work with various other projects.
>> >
>> > I personally like e-mail and 'hate' all kinds of various systems where I
>> > have to log in with different credentials.
>>
>> I totally agree with Wido, besides the review board workflow has a serious ownership issue:
>>
>> When I submit a git formatted patch it would strip the commit
>> message and author info, signature and retain only the unified diff.
>> The reviewer and/or committer have to do the extra work of
>> downloading the patch, applying/verifying it and then committing it.
>> During the process they may change the original commit message and
>> author signature (and they lose their ohloh points :)
>>
>> Further, the contributor is then required to go back and close the
>> submission. Well one can use their mailbox from where they can
>> import the patches, git am works. Or when the committer is
>> downloading the diff anyway, why not download the actual git
>> formatted patch emailed by an author and git apply? Just a
>> suggestion.
>>
>
> This was tried in the past and backfired when non-committers send
> through patches that get formatted by mail clients and have CRLF
> issues when applied by the committer.
>
> I agree Reviewboard has extra workflow involved but it integrates the
> review comments closely with the mailing list so it isn't as different
> from patches in the email in my opinion. It additionally provides
> pretty diff tools online. If not I would have to put the patch through
> my own diff tool.
>
> The current pain points as I see are :
>
> 1) ReviewBoard removes the author information from the diff
> 2) git am doesn't always work
> 3) extra workflow step of submitter closing the patch request
>
> These probably should be addressed by tooling.
I'm still very new to the project, but I found it very difficult to
follow up what's going on because patches are in separate places.
I'd argue there should be one single workflow so that we can better
keep track of the status.
Many of you might know, but post-review tool makes it easy to submit a patch:
http://www.reviewboard.org/docs/manual/dev/users/tools/post-review/
Tomoe
>
>>
>> >
>> > If somebody finds a typo or small bug, should they really go through all
>> > the hoops for submitting a small patch?
>> >
>> > Imho that shouldn't be necessary.
>> >
>> > Wido
>
> --
> Prasanna.,
Re: non-committer workflow
Posted by David Nalley <da...@gnsa.us>.
On Wed, Aug 1, 2012 at 12:47 PM, sebgoa <ru...@gmail.com> wrote:
>
> On Aug 1, 2012, at 10:29 AM, Joe Brockmeier wrote:
>>>
>>
>> It's different in that the submitter has to go to Review Board and
>> create an account, etc.
>>
>> If it's a one-off minor patch, a fair number of people are likely to
>> just say "meh" and drop it. Forcing people to use Review Board means
>> we'll lose some number of contributions.
>>
>> I'd suggest a better way is to heavily encourage Review Board as the
>> "official" submission method, and quietly allow email or attachments to
>> bugs.
>
>
> This seems to be the conclusion of this thread, even though review board is not perfect.
> We can edit the wiki page to clarify the non-committer workflow, and document review board process as well as "wido's" email workflow...
>
Edit boldly!
Re: non-committer workflow
Posted by sebgoa <ru...@gmail.com>.
On Aug 1, 2012, at 10:29 AM, Joe Brockmeier wrote:
>>
>
> It's different in that the submitter has to go to Review Board and
> create an account, etc.
>
> If it's a one-off minor patch, a fair number of people are likely to
> just say "meh" and drop it. Forcing people to use Review Board means
> we'll lose some number of contributions.
>
> I'd suggest a better way is to heavily encourage Review Board as the
> "official" submission method, and quietly allow email or attachments to
> bugs.
This seems to be the conclusion of this thread, even though review board is not perfect.
We can edit the wiki page to clarify the non-committer workflow, and document review board process as well as "wido's" email workflow...
Agreed ?
-sebastien
>
> Best,
>
> Joe
> --
> Joe Brockmeier
> http://dissociatedpress.net/
> Twitter: @jzb
Re: non-committer workflow
Posted by Joe Brockmeier <jz...@zonker.net>.
On Wed, Aug 01, 2012 at 06:26:38PM +0530, Prasanna Santhanam wrote:
> On Wed, Aug 01, 2012 at 08:42:51AM -0400, Rohit Yadav wrote:
> I agree Reviewboard has extra workflow involved but it integrates the
> review comments closely with the mailing list so it isn't as different
> from patches in the email in my opinion. It additionally provides
> pretty diff tools online. If not I would have to put the patch through
> my own diff tool.
It's different in that the submitter has to go to Review Board and
create an account, etc.
If it's a one-off minor patch, a fair number of people are likely to
just say "meh" and drop it. Forcing people to use Review Board means
we'll lose some number of contributions.
I'd suggest a better way is to heavily encourage Review Board as the
"official" submission method, and quietly allow email or attachments to
bugs.
Best,
Joe
--
Joe Brockmeier
http://dissociatedpress.net/
Twitter: @jzb
Re: non-committer workflow
Posted by Prasanna Santhanam <Pr...@citrix.com>.
On Wed, Aug 01, 2012 at 08:42:51AM -0400, Rohit Yadav wrote:
>
> On Aug 1, 2012, at 5:46 PM, Wido den Hollander <wi...@widodh.nl> wrote:
>
> > On 08/01/2012 04:50 AM, Alex Huang wrote:
> >>> So currently, there are three ways for a patch to be received:
> >>> 1. Email (see the workflow Wido proposed) 2. Reviewboard 3. Submitted
> >>> with a bug.
> >>>
> >>> Email and ReviewBoard are the most visible, and it seems most people are
> >>> using ReviewBoard rather than email.
> >>
> >> We should remove the email and submit with a bug options.
> >>
> >
> > Are you sure? For larger patches I agree that e-mail isn't that easy,
> > but it seems to work with various other projects.
> >
> > I personally like e-mail and 'hate' all kinds of various systems where I
> > have to log in with different credentials.
>
> I totally agree with Wido, besides the review board workflow has a serious ownership issue:
>
> When I submit a git formatted patch it would strip the commit
> message and author info, signature and retain only the unified diff.
> The reviewer and/or committer have to do the extra work of
> downloading the patch, applying/verifying it and then committing it.
> During the process they may change the original commit message and
> author signature (and they lose their ohloh points :)
>
> Further, the contributor is then required to go back and close the
> submission. Well one can use their mailbox from where they can
> import the patches, git am works. Or when the committer is
> downloading the diff anyway, why not download the actual git
> formatted patch emailed by an author and git apply? Just a
> suggestion.
>
This was tried in the past and backfired when non-committers send
through patches that get formatted by mail clients and have CRLF
issues when applied by the committer.
I agree Reviewboard has extra workflow involved but it integrates the
review comments closely with the mailing list so it isn't as different
from patches in the email in my opinion. It additionally provides
pretty diff tools online. If not I would have to put the patch through
my own diff tool.
The current pain points as I see are :
1) ReviewBoard removes the author information from the diff
2) git am doesn't always work
3) extra workflow step of submitter closing the patch request
These probably should be addressed by tooling.
>
> >
> > If somebody finds a typo or small bug, should they really go through all
> > the hoops for submitting a small patch?
> >
> > Imho that shouldn't be necessary.
> >
> > Wido
--
Prasanna.,
Re: non-committer workflow
Posted by Rohit Yadav <ro...@citrix.com>.
On Aug 1, 2012, at 5:46 PM, Wido den Hollander <wi...@widodh.nl> wrote:
> On 08/01/2012 04:50 AM, Alex Huang wrote:
>>> So currently, there are three ways for a patch to be received:
>>> 1. Email (see the workflow Wido proposed) 2. Reviewboard 3. Submitted
>>> with a bug.
>>>
>>> Email and ReviewBoard are the most visible, and it seems most people are
>>> using ReviewBoard rather than email.
>>
>> We should remove the email and submit with a bug options.
>>
>
> Are you sure? For larger patches I agree that e-mail isn't that easy,
> but it seems to work with various other projects.
>
> I personally like e-mail and 'hate' all kinds of various systems where I
> have to log in with different credentials.
I totally agree with Wido, besides the review board workflow has a serious ownership issue:
When I submit a git formatted patch it would strip the commit message and author info, signature and retain only the unified diff. The reviewer and/or committer have to do the extra work of downloading the patch, applying/verifying it and then committing it. During the process they may change the original commit message and author signature (and they lose their ohloh points :)
Further, the contributor is then required to go back and close the submission. Well one can use their mailbox from where they can import the patches, git am works. Or when the committer is downloading the diff anyway, why not download the actual git formatted patch emailed by an author and git apply? Just a suggestion.
Regards,
Rohit
>
> If somebody finds a typo or small bug, should they really go through all
> the hoops for submitting a small patch?
>
> Imho that shouldn't be necessary.
>
> Wido
Re: non-committer workflow
Posted by Wido den Hollander <wi...@widodh.nl>.
On 08/01/2012 04:50 AM, Alex Huang wrote:
>> So currently, there are three ways for a patch to be received:
>> 1. Email (see the workflow Wido proposed) 2. Reviewboard 3. Submitted
>> with a bug.
>>
>> Email and ReviewBoard are the most visible, and it seems most people are
>> using ReviewBoard rather than email.
>
> We should remove the email and submit with a bug options.
>
Are you sure? For larger patches I agree that e-mail isn't that easy,
but it seems to work with various other projects.
I personally like e-mail and 'hate' all kinds of various systems where I
have to log in with different credentials.
If somebody finds a typo or small bug, should they really go through all
the hoops for submitting a small patch?
Imho that shouldn't be necessary.
Wido
RE: non-committer workflow
Posted by Alex Huang <Al...@citrix.com>.
> So currently, there are three ways for a patch to be received:
> 1. Email (see the workflow Wido proposed) 2. Reviewboard 3. Submitted
> with a bug.
>
> Email and ReviewBoard are the most visible, and it seems most people are
> using ReviewBoard rather than email.
We should remove the email and submit with a bug options.
--Alex
Re: non-committer workflow
Posted by David Nalley <da...@gnsa.us>.
On Tue, Jul 31, 2012 at 5:05 PM, sebgoa <ru...@gmail.com> wrote:
>
> So Non-committers don't need to get an account on RB and submit patches via that path ?
>
So currently, there are three ways for a patch to be received:
1. Email (see the workflow Wido proposed)
2. Reviewboard
3. Submitted with a bug.
Email and ReviewBoard are the most visible, and it seems most people
are using ReviewBoard rather than email.
--David
Re: non-committer workflow
Posted by sebgoa <ru...@gmail.com>.
On Jul 31, 2012, at 4:36 PM, Wido den Hollander wrote:
> On 07/31/2012 10:24 PM, sebgoa wrote:
>> Hi,
>>
>> Since I am new to Cloudstack and Apache like projects, I was looking at the process for submitting a very small patch (e.g typo, one-liner, license header)
>
> Always welcome!
>
>>
>> The wiki page on how to use git o contribute to Cloudstack has a section for non-committer:
>>
>> http://wiki.cloudstack.org/display/dev/Git+workflow+in+the+brave+new+world#Gitworkflowinthebravenewworld-Non-committerworkflow
>>
>> This is a bit confusing to me, so I would like to get some feedback from the more seasoned developper and committers:
>>
>> 1-Does every patch needs to be submitted in response to a filed bug ?
>
> No, I don't think it needs to. If it's a trivial typo fix it's not needed.
>
>> 2-Do we got through https://reviews.apache.org ?
So Non-committers don't need to get an account on RB and submit patches via that path ?
>> 3-The wiki mentions sending the patch to dev mailing list with subject [PATCH] but that's not what I have seen in the last few weeks ?
>
> Committers don't need to sent their patches over the dev mailinglist. We haven't seen much external code lately.
>
>> 4-What's the best way to use git to prepare a small patch (e.g docs typo, one-liner ...etc), do we really need to create a branch ? how does it get resolved locally once the patch is accepted ?
>
> Yes, you still have to branch. The commands:
>
> $ git clone https://git-wip-us.apache.org/repos/asf/incubator-cloudstack.git
> $ git checkout -b typo-fix
> $ vi/nano/vim <file to fix>
> $ git commit <file to fix>
> $ git format-patch -s master
> $ git send-email newly-generated-patch-0001.patch
>
> Somebody with commit access will pick up that patch and apply if it's acceptable.
>
> Wido
>
>>
>> It would be nice to agree on a clear worfklow for non-committers (and newbies like me) so we can get some people in the mix and graduate to committers...
>>
>> Thoughts anyone ?
>>
>> Thanks,
>>
>> -Sebastien Goasguen
>> Apache Cloudstack Evangelist
>> https://sites.google.com/site/runseb/
>>
>>
Re: non-committer workflow
Posted by Wido den Hollander <wi...@widodh.nl>.
On 07/31/2012 10:24 PM, sebgoa wrote:
> Hi,
>
> Since I am new to Cloudstack and Apache like projects, I was looking at the process for submitting a very small patch (e.g typo, one-liner, license header)
Always welcome!
>
> The wiki page on how to use git o contribute to Cloudstack has a section for non-committer:
>
> http://wiki.cloudstack.org/display/dev/Git+workflow+in+the+brave+new+world#Gitworkflowinthebravenewworld-Non-committerworkflow
>
> This is a bit confusing to me, so I would like to get some feedback from the more seasoned developper and committers:
>
> 1-Does every patch needs to be submitted in response to a filed bug ?
No, I don't think it needs to. If it's a trivial typo fix it's not needed.
> 2-Do we got through https://reviews.apache.org ?
> 3-The wiki mentions sending the patch to dev mailing list with subject [PATCH] but that's not what I have seen in the last few weeks ?
Committers don't need to sent their patches over the dev mailinglist. We
haven't seen much external code lately.
> 4-What's the best way to use git to prepare a small patch (e.g docs typo, one-liner ...etc), do we really need to create a branch ? how does it get resolved locally once the patch is accepted ?
Yes, you still have to branch. The commands:
$ git clone https://git-wip-us.apache.org/repos/asf/incubator-cloudstack.git
$ git checkout -b typo-fix
$ vi/nano/vim <file to fix>
$ git commit <file to fix>
$ git format-patch -s master
$ git send-email newly-generated-patch-0001.patch
Somebody with commit access will pick up that patch and apply if it's
acceptable.
Wido
>
> It would be nice to agree on a clear worfklow for non-committers (and newbies like me) so we can get some people in the mix and graduate to committers...
>
> Thoughts anyone ?
>
> Thanks,
>
> -Sebastien Goasguen
> Apache Cloudstack Evangelist
> https://sites.google.com/site/runseb/
>
>