If it's mandated by Apache rules, I understand, but I do think that GitHub/git provide improved workflow over SVN + patch. Apache appears to be mirroring to GitHub anyway:
https://github.com/apache/kafka You even have a pull request (5 months old) already. Things like pull request review/commenting, as mentioned, are very nice, and it would be a shame to not be able to use them. Derek On Mon, Jan 7, 2013 at 9:06 AM, Jay Kreps <jay.kr...@gmail.com> wrote: > The reason we take diffs is because traditionally the mandatory Apache > toolchain is svn+jira+patch/diff. When we were on github of course we used > that. > > I'm actually not sure of the Apache rules here. Can we just directly accept > github pull requests? I.e. you fork the apache mirror and send a pull > request? Github has lots of tools for seeing diffs, commenting on code, etc > so this would be fantastic. Is that considered bad form? We could just have > the JIRA point to the github url... > > -Jay > > > On Mon, Jan 7, 2013 at 7:05 AM, Derek Chen-Becker <de...@precog.com> > wrote: > > > 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> > > >>> > > > >>> */ > > >>> > > >>> > > >> > > >> > > > > > > -- *Derek Chen-Becker* *Precog Lead Infrastructure Engineer* de...@precog.com 303-752-1700
