Hey folks, So I'm trying to get in a patch but this has not been updated to tell committers how to actually get the git keys set up: https://cwiki.apache.org/confluence/display/ZOOKEEPER/Committing+changes
Can someone float me a link that says how to do this? Also a bunch of our documentation still discusses SVN and not git, which means we are not done with this migration. If you were pushing for this change can you please do some due diligence on the wikis and correct the stuff that refers to SVN? Thanks, C On Wed, Oct 5, 2016 at 1:02 PM, Edward Ribeiro <[email protected]> wrote: > Excuse me guys! > > I've written on Macbook Pro. No idea why GMail messed it up. I was only > able to see the strange characters when I pasted on a gist text area. The > previous message is below, but if anyone is still having trouble (I tried > to remove the weird character), I left a copy at: > https://gist.github.com/eribeiro/da2a6a6c9a508610d52d0755fae8352d > > "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. > > There 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, not every Kafka PR needs a corresponding JIRA ticket. There are a > class of PRs 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 leaves up to the contributors to either open a GH PR or upload the > patch file to JIRA. > > > +1 about having a 'paper trail' of review comments on 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 Wed, Oct 5, 2016 at 1:35 PM, Michael Han <[email protected]> wrote: > > > Eddie's mail contains lots of '=E2=80=8B'' which is unicode character > > zero-width space, which might cause parsing trouble for some mail > clients. > > > > On Wed, Oct 5, 2016 at 6:42 AM, Flavio Junqueira <[email protected]> wrote: > > > > > Dude, I'm just not able to parse your e-mail, did you write that on a > > > phone or something? > > > > > > -Flavio > > > > > > > On 05 Oct 2016, at 03:54, Edward Ribeiro <[email protected]> > > > wrote: > > > > > > > > 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 <[email protected]> > 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 <[email protected]> > > > 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 <[email protected]> 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 < > > > >>> [email protected]> > > > >>>> 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 < > [email protected]> > > > >>> 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 < > > [email protected] > > > > > > > >>>>>> 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 < > [email protected] > > > > > > >>>>> wrote: > > > >>>>>>> > > > >>>>>>>> On Mon, Sep 19, 2016 at 4:11 PM, Benjamin Reed < > > [email protected]> > > > >>>>>> 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 < > > [email protected]> > > > >>>>>> 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 < > > > [email protected] > > > >>> > > > >>>>>>>> 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. > > > >> > > > > > > > > > > > > -- > > Cheers > > Michael. > > >
