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 <f...@apache.org> 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 <edward.ribe...@gmail.com> > 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 <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. > >> > > -- Cheers Michael.
