It makes it easier for a non-committer to contribute via email, but with publicly available repos (a la GitHub) it's just as easy to merge from a remote (and doesn't require contorting through hoops for certain scenarios). On Jan 7, 2013 7:45 AM, "David Arthur" <mum...@gmail.com> wrote:
> Diff/patch makes it easy for non-committer to contribute. > > On 1/7/13 12:52 AM, Derek Chen-Becker wrote: > >> Although I haven't contributed much here yet, I did want to ask: why >> diff/patch and not pull/merge? I know my work on getting the SBT build >> working with a modern SBT was quite a headache for everyone because the >> diff format was unable to convey things like "delete this binary file and >> add this other one". >> >> Derek >> >> >> On Sat, Jan 5, 2013 at 10:35 PM, Joe Stein <crypt...@gmail.com> wrote: >> >> ok with some more research today it seems the difference and issues I was >>> having was from the patch being made with >>> >>> git diff vs git format-patch >>> >>> with git diff (which is how the patch I was reviewing was made) you apply >>> doing "patch -p1 < patch" >>> >>> no commits messages are preserved with git diff. I think there are pros >>> and cons to this. >>> >>> If folks make good commit messages then this is great however I prefer >>> the >>> git diff patch myself from contribs because then I can commit with a >>> message for the JIRA ticket and the reviewer >>> >>> thoughts on git diff vs git format-patch ? >>> >>> I updated the wiki to deal with the error i encountered since it already >>> references format-patch I but think we should have some consensus for >>> contributors and how they should proceed and how we should too. >>> >>> On Sat, Jan 5, 2013 at 2:02 PM, Joe Stein <crypt...@gmail.com> wrote: >>> >>> Ok, I figured out the problem. The problem was with the patch format so >>>> >>> I >>> >>>> will take care of that ... the patch is minor enough I will take the >>>> code >>>> changes and whip up a new patch and let Maxime know (assuming that patch >>>> >>> is >>> >>>> good) about how to make a Kafka patch moving forward). >>>> >>>> I noticed the incubation URL was wrong on the README so I walked through >>>> the contributor steps and everything worked just perfectly >>>> >>>> the only thing I did notice is that the commit message I put in "as a >>>> contributor" was part of the patch and everything so I think we should >>>> >>> call >>> >>>> out some guidelines for making commit messages, like always put the >>>> KAFKA-XYZ in the message so when we review and push everything goes in >>>> >>> how >>> >>>> we expected if we made the change and committed ourselves. >>>> >>>> I just could not let it go, now I am going to-do what I need to for work >>>> before my daughter wakes up =8^) >>>> >>>> >>>> On Sat, Jan 5, 2013 at 1:42 PM, Joe Stein <crypt...@gmail.com> wrote: >>>> >>>> that did not work either >>>>> >>>>> I can't even get the patch to apply from the latest trunk because of >>>>> >>>> this >>> >>>> message of patch without email >>>>> >>>>> so the patch is here >>>>> >>>>> https://issues.apache.org/**jira/secure/attachment/** >>> 12563266/KAFKA-133.patch<https://issues.apache.org/jira/secure/attachment/12563266/KAFKA-133.patch> >>> >>>> I go through the steps on the git workflow >>>>> >>>>> git clone >>>>> https://git-wip-us.apache.org/**repos/asf/kafka.git<https://git-wip-us.apache.org/repos/asf/kafka.git>kafka >>>>> cd kafka >>>>> git fetch >>>>> git checkout trunk >>>>> //already on trunk >>>>> git apply --stat ../KAFKA-133.patch >>>>> //project/build.properties | 2 +- >>>>> //project/build/KafkaProject.**scala | 44 >>>>> +++++++++++++++++++++---------**-------- >>>>> //2 files changed, 25 insertions(+), 21 deletions(-) >>>>> git apply --check ../KAFKA-133.patch >>>>> git am --signoff < ../KAFKA-133.patch >>>>> //Patch does not have a valid e-mail address. >>>>> >>>>> my git --version = 1.8.0.3 >>>>> >>>>> now what is interesting is when I grab the patch using wget >>>>> >>>>> https://issues.apache.org/**jira/secure/attachment/** >>> 12563266/KAFKA-133.**patchinsteadof<https://issues.apache.org/jira/secure/attachment/12563266/KAFKA-133.patchinsteadof>downloading >>> it it through a browser I get "Patch format >>> >>>> detection failed." instead of the error saying "Patch does not have a >>>>> >>>> valid >>> >>>> e-mail address" >>>>> >>>>> I am guessing it is something I am doing wrong and could be doing >>>>> different but am interested to see where exactly the problem is. >>>>> >>>>> any thoughts? I gotta work on some code for work right will bang on >>>>> >>>> this >>> >>>> later tonight again but if anyone can reproduce this same thing or not >>>>> >>>> or >>> >>>> has an idea that would be great. >>>>> >>>>> could just be the patch, but would prefer to fix the patch and review >>>>> >>>> the >>> >>>> code change for what it is and communicate moving forward how to make >>>>> patches differently (if that is in fact the problem) >>>>> >>>>> >>>>> On Sat, Jan 5, 2013 at 12:38 PM, David Arthur <mum...@gmail.com> >>>>> wrote: >>>>> >>>>> You can amend the previous commit (as long as you havent pushed) with >>>>>> >>>>> an >>> >>>> author like "git --amend --author='...'" >>>>>> >>>>>> On Saturday, January 5, 2013, Joe Stein wrote: >>>>>> >>>>>> I am getting the no email after doing >>>>>>> >>>>>>> git am --signoff < xyz.patch >>>>>>> >>>>>>> so nothing gets in to commit to set the author :( >>>>>>> >>>>>>> On Sat, Jan 5, 2013 at 12:30 AM, Jay Kreps <jay.kr...@gmail.com >>>>>>> >>>>>> <javascript:;>> >>>>>> >>>>>>> wrote: >>>>>>> >>>>>>> I have but I don't really know why. This format worked for me: >>>>>>>> git commit --author='Bertrand Russell <bruss...@cambridge.edu >>>>>>>> >>>>>>> <javascript:;> >>>>>> >>>>>>> ' >>>>>>>> >>>>>>>> >>>>>>>> On Fri, Jan 4, 2013 at 8:35 PM, Joe Stein <crypt...@gmail.com >>>>>>>> >>>>>>> <javascript:;>> >>>>>> >>>>>>> wrote: >>>>>>> >>>>>>>> I started following this so far really helpful thanks!! >>>>>>>>> >>>>>>>>> Running into some issues reviewing someone's patch getting "Patch >>>>>>>>> >>>>>>>> does >>>>>> >>>>>>> not >>>>>>>> >>>>>>>>> have a valid e-mail address." googling to figure out what is >>>>>>>>> >>>>>>>> wrong >>> >>>> figure I >>>>>>>> >>>>>>>>> mention here if anyone bumped into this yet >>>>>>>>> >>>>>>>>> thnx >>>>>>>>> >>>>>>>>> On Thu, Jan 3, 2013 at 11:17 AM, Jun Rao <jun...@gmail.com >>>>>>>>> >>>>>>>> <javascript:;>> >>>>>> >>>>>>> wrote: >>>>>>> >>>>>>>> Thanks for documenting this. Could you also add how to resolve >>>>>>>>>> >>>>>>>>> conflicts >>>>>>>> >>>>>>>>> during rebase? >>>>>>>>>> >>>>>>>>>> Jun >>>>>>>>>> >>>>>>>>>> On Wed, Jan 2, 2013 at 1:45 PM, Jay Kreps <jay.kr...@gmail.com >>>>>>>>>> >>>>>>>>> <javascript:;>> >>>>>> >>>>>>> wrote: >>>>>>> >>>>>>>> I don't know about other people but I find git kind of >>>>>>>>>>> >>>>>>>>>> confusing. I >>>>>> >>>>>>> thought >>>>>>>>>> >>>>>>>>>>> it would be useful to try to document the normal workflow for >>>>>>>>>>> >>>>>>>>>> different >>>>>>>> >>>>>>>>> use >>>>>>>>>> >>>>>>>>>>> cases: >>>>>>>>>>> 1. Contributing a patch >>>>>>>>>>> 2. Reviewing and integrating a patch that is contributed >>>>>>>>>>> 3. Doing development as a committer >>>>>>>>>>> 4. Keeping a copy of your local work on github (since it >>>>>>>>>>> >>>>>>>>>> doesn't >>>>>> >>>>>>> seem >>>>>>> >>>>>>>> Apache has a place to keep local backups of work in >>>>>>>>>>> >>>>>>>>>> progress). >>> >>>> >>>>>>>>>>> https://cwiki.apache.org/**confluence/display/KAFKA/Git+** >>> Workflow<https://cwiki.apache.org/confluence/display/KAFKA/Git+Workflow> >>> >>>> I would like to link this up from the contributor page to >>>>>>>>>>> >>>>>>>>>> help >>> >>>> people >>>>>>> >>>>>>>> (including my future self). Objections? >>>>>>>>>>> >>>>>>>>>>> I am not a git expert, so any feedback to improve these >>>>>>>>>>> >>>>>>>>>> recipes or >>>>>> >>>>>>> bug >>>>>>>> >>>>>>>>> fixes (since I haven't tried everything) would be very much >>>>>>>>>>> >>>>>>>>>> appreciated. >>>>>>>>> >>>>>>>>>> If >>>>>>>>>> >>>>>>>>>>> you are about to do one of the above things, try the recipe >>>>>>>>>>> >>>>>>>>>> and see >>>>>> >>>>>>> if >>>>>>>> >>>>>>>>> it >>>>>>>>> >>>>>>>>>> can be improved. >>>>>>>>>>> >>>>>>>>>>> Cheers, >>>>>>>>>>> >>>>>>>>>>> -Jay >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>> >>>>>>>>> -- >>>>>>>>> >>>>>>>>> /* >>>>>>>>> Joe Stein >>>>>>>>> http://www.linkedin.com/in/**charmalloc<http://www.linkedin.com/in/charmalloc> >>>>>>>>> Twitter: @allthingshadoop < >>>>>>>>> >>>>>>>> http://www.twitter.com/**allthingshadoop<http://www.twitter.com/allthingshadoop> >>> > >>> >>>> */ >>>>>>>>> >>>>>>>>> >>>>>>> >>>>>>> -- >>>>>>> >>>>>>> /* >>>>>>> Joe Stein >>>>>>> http://www.linkedin.com/in/**charmalloc<http://www.linkedin.com/in/charmalloc> >>>>>>> Twitter: @allthingshadoop >>>>>>> <http://www.twitter.com/**allthingshadoop<http://www.twitter.com/allthingshadoop> >>>>>>> > >>>>>>> */ >>>>>>> >>>>>>> >>>>>> -- >>>>>> David Arthur >>>>>> >>>>>> >>>>> >>>>> -- >>>>> >>>>> /* >>>>> Joe Stein >>>>> http://www.linkedin.com/in/**charmalloc<http://www.linkedin.com/in/charmalloc> >>>>> Twitter: @allthingshadoop >>>>> <http://www.twitter.com/**allthingshadoop<http://www.twitter.com/allthingshadoop> >>>>> > >>>>> */ >>>>> >>>>> >>>> >>>> -- >>>> >>>> /* >>>> Joe Stein >>>> http://www.linkedin.com/in/**charmalloc<http://www.linkedin.com/in/charmalloc> >>>> Twitter: @allthingshadoop >>>> <http://www.twitter.com/**allthingshadoop<http://www.twitter.com/allthingshadoop> >>>> > >>>> */ >>>> >>>> >>> >>> -- >>> >>> /* >>> Joe Stein >>> http://www.linkedin.com/in/**charmalloc<http://www.linkedin.com/in/charmalloc> >>> Twitter: @allthingshadoop >>> <http://www.twitter.com/**allthingshadoop<http://www.twitter.com/allthingshadoop> >>> > >>> */ >>> >>> >> >> >
