kafka-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Derek Chen-Becker <de...@precog.com>
Subject Re: git workflow
Date Mon, 07 Jan 2013 16:32:57 GMT
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.kreps@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 <derek@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" <mumrah@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 <cryptcom@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 <cryptcom@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 <cryptcom@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 <mumrah@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.kreps@gmail.com
> > >>>>>>>
> > >>>>>> <javascript:;>>
> > >>>>>>
> > >>>>>>> wrote:
> > >>>>>>>
> > >>>>>>>  I have but I don't really know why. This format worked
for me:
> > >>>>>>>>    git commit --author='Bertrand Russell <
> brussell@cambridge.edu
> > >>>>>>>>
> > >>>>>>> <javascript:;>
> > >>>>>>
> > >>>>>>> '
> > >>>>>>>>
> > >>>>>>>>
> > >>>>>>>> On Fri, Jan 4, 2013 at 8:35 PM, Joe Stein <cryptcom@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 <junrao@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.kreps@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*
derek@precog.com
303-752-1700

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message