Hi, Now we've moved to git, what is the policy for uploading patches and doing code reviews? I am asking because I've seen recently there are git pull requests coming in without associated patch file uploaded to JIRA. I've checked https://cwiki.apache.org/confluence/display/ZOOKEEPER/HowToContribute, looks like there is not much change regarding patch process - so presumably we still need to generate and upload patch file to JIRA for the record, while using github (maybe in addition of review board, or in the future with gerrit) to do code reviews?
On Wed, Sep 21, 2016 at 6:05 AM, Edward Ribeiro <edward.ribe...@gmail.com> wrote: > Cool, just open https://issues.apache.org/jira/browse/ZOOKEEPER-2597 > > PS: I removed the REPO_HOME global variable. > > On Wed, Sep 21, 2016 at 6:53 AM, Flavio Junqueira <f...@apache.org> wrote: > > > Better to have that in the form of a pull request or diff. > > > > REPO_HOME does seem to be unused. > > > > -Flavio > > > > > On 20 Sep 2016, at 18:57, Edward Ribeiro <edward.ribe...@gmail.com> > > wrote: > > > > > > Hey, I have started porting the kafka-merge.py to work on ZK repos. I > > would > > > need someone to review it and help me test it now. > > > > > > The files were uploaded below, but I will create a github repo yet > today. > > > > > > https://www.dropbox.com/sh/od8bet2574jttm3/ > > AADv1DXTb8vfyVCmelFbYCEha?dl=0 > > > > > > I uploaded the kafka version script so that you can use diff or Meld to > > > spot my changes, but feel free to grasp the original file here: > > > https://github.com/apache/kafka/blob/trunk/kafka-merge-pr.py > > > > > > PS: It's just me or REPO_HOME env variable is not used anywhere in the > > > merge script??? > > > > > > Cheers, > > > Eddie > > > > > > On Tue, Sep 20, 2016 at 12:19 PM, Patrick Hunt <ph...@apache.org> > wrote: > > > > > >> On Mon, Sep 19, 2016 at 4:11 PM, Benjamin Reed <br...@apache.org> > > wrote: > > >> > > >>> what you are suggesting sounds good, but i don't know how to do it? > > since > > >>> in the end we are still just accepting diffs on patches, the only > thing > > >>> that changes is that we use svn rather than git right? > > >>> > > >>> > > >> Notice the workflow Kafka uses - which includes "git apply" and > > specifying > > >> the author tag when committers commit (so that the OP gets proper > > >> attribution in the commit itself) > > >> > > >> https://cwiki.apache.org/confluence/display/KAFKA/ > > Manual+Commit+Workflow > > >> > > >> Patrick > > >> > > >> > > >> > > >>> i LOVE chris's idea! lets do it! > > >>> > > >>> ben > > >>> > > >>> On Sun, Sep 18, 2016 at 3:22 PM, Patrick Hunt <ph...@apache.org> > > wrote: > > >>> > > >>>> Ben, do you also want to update the "Applying a patch" section to > make > > >> it > > >>>> git specific? > > >>>> > > >>>> We (committers) should move to a model where authors get proper > credit > > >> in > > >>>> git. Our old workflow in svn resulted in only the committer being > > >> listed > > >>>> (except that we listed the patch author in the commit message). We > > >> should > > >>>> move to a model where the author of the patch gets proper credit in > > >> git. > > >>> I > > >>>> believe we will get that if we use git for patch > creation/application? > > >>>> > > >>>> Chris brought up getting rid of CHANGES.txt recently on the dev list > > >> in a > > >>>> separate thread - Chris do you want to implement that change now > that > > >>> we've > > >>>> moved to git? > > >>>> > > >>>> Patrick > > >>>> > > >>>> On Wed, Sep 14, 2016 at 9:01 PM, Benjamin Reed <br...@apache.org> > > >> wrote: > > >>>> > > >>>>>> 1) actually in the previous step that was just adding new files. > you > > >>>>>> still > > >>>>>>> need the commit -a for the rest of the changes. that's my normal > > >>>>>> workflow. > > >>>>>> > > >>>>>> I think that will be confusing for most folks. They typically > stage > > >>>>>> all the changes and then commit or don't stage and use -a. > > >>>>>> > > >>>>> > > >>>>> do you mind fixing it with your workflow. commit -a doesn't get new > > >>>>> files, which is why you need to do the add, but i'm not the most > > >>>>> sophisticated git user, so > > >>>>> > > >>>>> > > >>>>>> > > >>>>>>> 2) i figured since we are using git now that we should use git's > > >>>>>> default. > > >>>>>>> the patch should work (by default it seems to strip the first > path > > >>>>>> element). > > >>>>>>> does it not work for you? > > >>>>>>> > > >>>>>> > > >>>>>> It will fail precommit in it's current state. > > >>>>>> > > >>>>> > > >>>>> fixed > > >>>>> > > >>>> > > >>>> > > >>> > > >> > > > > > -- Cheers Michael.
