Hi, The patch attached on ZOOKEEPER-2597 is a straightforward adaptation of Kafka's one. It takes care of merging Github PR into Apache git repo and a subsequent closing of the PR on the GH side among other things (rewriting of commit messages, etc) . The current status is: the script needs to be reviewed/validated by a committer. It has been some time since I uploaded the patch, so I gonna do another pass through it in the meantime.
T here are some workflow issues beyond the scope of ZOOKEEPER-2597 that need to be sorted out (IMO) : 1. The normal workflow is to open a JIRA ticket before doing any GH PR , that is, no JIRA-less PRs. The PR should have a title of the form "ZOOKEEPER-xxxx: Title". This will trigger the Apache JIRA-Github integration and it's opening show up in the JIRA ticket. 2. OTOH, n ot every Kafka PR needs a corresponding JIRA ticket . There are a class of PR s with "MINOR" title that represent trivial code changes and "HOT-FIX" title that fix urgent, but simple bugs. Both bypass the JIRA creation step , even tough they are still subject to review . It's worth adopting a similar approach for ZK project? 3. IIRC (didn't find any page to confirm) , Cassandra project encourages, but not demands, that contributors also upload a patch file to JIRA even in the case of a GH PR (as to leave a audit trail, I guess) . Or , at least , C* project leave s up to the contributor s to either open a GH PR or upload the patch file to JIRA. In fact, Github allows the access to a raw patch or diff, it's just a matter of adding the ".patch" or ".diff" suffix to the end of the Pull Request URL. +1 about having a 'paper trail' of review comments o n JIRA and /or mailing list (I would prefer the mailing list tbh). But as Michael and Flavio pointed out, I never seen GH PR review ** comments ** being written back to JIRA, at least not in Kafka, Cassandra or Solr projects , that I have followed more closely. Eddie On Tue, Oct 4, 2016 at 6:40 PM, Michael Han <h...@cloudera.com> wrote: > >> as long as the opening/closing/commenting all get sent to the mailing > list or recorded in jira > Yeah, this is my impression as well, that we need to keep certain paper > trail regarding activities and comments on ASF side (JIRA or mail list). > Infra page said: > > - Any Pull Request that gets opened, closed, reopened or **commented** > on now gets recorded on the project's mailing list > - If a project has a JIRA instance, any PRs or *comments* on PRs that > include a JIRA ticket ID will trigger an update on that specific ticket > > I checked a couple Kafka and Spark JIRAs but I don't see any of the > comments made in github PR were posted on JIRA, except the activities (open > a PR, close a PR). Since both projects have been using github for a while I > assume such practice of NOT integrating comments between github and ASF > JIRA is acceptable? Though I feel it would be really useful if comments > could converge in a single place as well, that will provide a clear history > for a given technical issue. > > On Tue, Oct 4, 2016 at 12:06 PM, Flavio Junqueira <f...@apache.org> wrote: > > > Until ZOOKEEPER-2597 <https://issues.apache.org/ > jira/browse/ZOOKEEPER-2597> > > is fixed, we can't merge via github. > > > > For code reviews, we can use GH as long as the opening/closing/commenting > > all get sent to the mailing list or recorded in jira. I don't think we > have > > that yet, but it is possible according to this: > > > > https://blogs.apache.org/infra/entry/improved_ > > integration_between_apache_and <https://blogs.apache.org/ > > infra/entry/improved_integration_between_apache_and> > > > > For now, we do need to upload patches and converge using jira. > > > > I think Eddie has been looking at this process trying to replicate the > > Kafka setup, so perhaps he can give an update if I'm right. Kafka doesn't > > send every comment to the mailing list, though, but I'm not sure if > that's > > acceptable according to the ASF, I need to double-check. > > > > -Flavio > > > > > On 04 Oct 2016, at 19:42, Michael Han <h...@cloudera.com> wrote: > > > > > > 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. > > > > > > > -- > Cheers > Michael. >
