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
I go through the steps on the git workflow

git clone 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 
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
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
Twitter: @allthingshadoop <
http://www.twitter.com/allthingshadoop>
*/



--

/*
Joe Stein
http://www.linkedin.com/in/charmalloc
Twitter: @allthingshadoop <http://www.twitter.com/allthingshadoop>
*/


--
David Arthur



--

/*
Joe Stein
http://www.linkedin.com/in/charmalloc
Twitter: @allthingshadoop <http://www.twitter.com/allthingshadoop>
  */



--

/*
Joe Stein
http://www.linkedin.com/in/charmalloc
Twitter: @allthingshadoop <http://www.twitter.com/allthingshadoop>
*/



--

/*
Joe Stein
http://www.linkedin.com/in/charmalloc
Twitter: @allthingshadoop <http://www.twitter.com/allthingshadoop>
*/




Reply via email to