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 15:05:52 GMT
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>
>>> >
>>> */
>>>
>>>
>>
>>
>

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