zookeeper-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Flavio Junqueira <...@apache.org>
Subject Re: [VOTE] move Apache Zookeeper to git
Date Wed, 05 Oct 2016 13:42:08 GMT
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.ribeiro@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 <hanm@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 <fpj@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 <hanm@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.ribeiro@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 <fpj@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.ribeiro@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 <phunt@apache.org>
>>>>> wrote:
>>>>>>> 
>>>>>>>> On Mon, Sep 19, 2016 at 4:11 PM, Benjamin Reed <breed@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 <phunt@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 <breed@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.
>> 


Mime
View raw message