ambari-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jonathan Hurley <jhur...@hortonworks.com>
Subject Re: [DISCUSS] Future code review and commit process
Date Mon, 08 Jan 2018 22:25:41 GMT
My vote would be to open a new pull request - if anything just to get more practice. Even if
you already have +1's, it's good to open it and reference the reviewboard.

On Jan 8, 2018, at 2:00 PM, Attila Doroszlai <adoroszlai@hortonworks.com<mailto:adoroszlai@hortonworks.com>>
wrote:

Is there a policy for pending review requests (ie. the ones already open at https://reviews.apache.org/groups/Ambari/
)?  Should we open a PR for each, or should they be wrapped up on Review Board, or is it up
to us?

Thanks.

-Attila

From: Vivek Ratnavel <vivekratnavel@apache.org<mailto:vivekratnavel@apache.org><mailto:vivekratnavel@apache.org>>
Reply-To: "dev@ambari.apache.org<mailto:dev@ambari.apache.org><mailto:dev@ambari.apache.org>"
<dev@ambari.apache.org<mailto:dev@ambari.apache.org><mailto:dev@ambari.apache.org>>
Date: Friday, January 5, 2018 at 12:16 AM
To: "dev@ambari.apache.org<mailto:dev@ambari.apache.org><mailto:dev@ambari.apache.org>"
<dev@ambari.apache.org<mailto:dev@ambari.apache.org><mailto:dev@ambari.apache.org>>
Subject: Re: [DISCUSS] Future code review and commit process

Further clarifications:

- Creating the fork
https://help.github.com/articles/fork-a-repo/

- Creating a branch for every commit (and creating the pull request)
https://help.github.com/articles/creating-a-pull-request-from-a-fork/

- How to keep your fork in-sync with the upstream repository

https://help.github.com/articles/syncing-a-fork/

- How long-lived Apache feature branches will work in this model. In this case, we'd still
need a branch off of the feature branch for every commit from the fork.

In this workflow, a feature branch is no different than any other branch. If you want a commit
to land in a branch, then you create a new branch based off that branch. You create multiple
branches if you want your commits to land in multiple branches. It might sound like a daunting
task initially, but believe me, its very easy and straightforward to create a branch and open
pull requests for review. And once a pull request is opened, you can make changes by simply
pushing commits to the same branch.

- How to merge long-lived feature branches into Apache

Feature branches or any other branch could be merged with trunk or any branch by creating
a new pull request. A new pull request could be opened by selecting two branches - a base
branch and a head branch. In this case, if you want to merge a feature branch with trunk,
then you select feature branch as base branch and trunk as head branch.

I have attached a screen-shot for reference.

[cid:ii_jc13wnyl0_160c3724ae47072e]


I agree with you on creating a wiki page to cover all the scenarios.

​
Thanks,
Vivek Ratnavel



On Thu, Jan 4, 2018 at 1:05 PM, Jonathan Hurley <jhurley@hortonworks.com<mailto:jhurley@hortonworks.com><mailto:jhurley@hortonworks.com>>
wrote:
Thanks for the clarifications. This sounds like the "Forking Workflow" as opposed to the "Feature
Branch Workflow". I'm fine with that since it lets non-commiters help.

We should try to capture all of these scenarios in a wiki page which we can then all agree
upon. Things which we need to cover are:

- Creating the fork
- Creating a branch for every commit (and creating the pull request)
- How to keep your fork in-sync with the upstream repository
- How long-lived Apache feature branches will work in this model. In this case, we'd still
need a branch off of the feature branch for every commit from the fork.
- How to merge long-lived feature branches into Apache

A few of the items above haven't been specified yet - like keeping the forked repo in sync
and how to manage long-lived feature branches in Apache.

I still do not think we need [component-1][component-2] in the commit message. We can use
the fields in Apache Jira for this. It makes our commit messages long, hard to read, and ugly.

On Jan 4, 2018, at 12:57 PM, Vivek Ratnavel <vivekratnavel@apache.org<mailto:vivekratnavel@apache.org><mailto:vivekratnavel@apache.org>>
wrote:

Let me clarify a few things here.


 - Before opening any pull requests, one needs to fork
 https://github.com/apache/ambari. This is a one time process.
 - Before working on any JIRA, lets say AMBARI-12345, one needs to create
 a branch from their own fork. Everyone can have their own naming
 conventions to name this branch since this is not going to affect the
 public repository in any way.
 - To answer Nate's question, if a JIRA has to be committed to branch-2.6
 and trunk, one needs to create two branches from their own fork - a branch
 based on branch-2.6 and another branch based on trunk. Let's name them
 AMBARI-12345-branch-2.6 and AMBARI-12345-trunk. Again this could be
 anything as long as you can differentiate.
 - After committing patches to both the newly created branches, you need
 to open two pull requests against two public branches - branch2.6 and
 trunk. This link should help -
 https://help.github.com/articles/creating-a-pull-request-from-a-fork/
 - If there is no conflict, github offers "squash and merge" option which
 will let you remove unnecessary commit messages and merge any number of
 commits as one commit. For more info -
 https://help.github.com/articles/about-pull-request-merges

Hope this clarifies the flow.

To clarify Jonathan's suggestion

* I do not think that adding a [COMPONENT] tag is useful. Many commits span
ambari-server and ambari-agent, and a good number also span ambari-web and
ambari-server. I also think that we should have the title of the JIra match
the commit exactly as we do today.

If a commit spans multiple components, lets say ambari-server and
ambari-web, the PR title should be [AMBARI-12345][ambari-server][ambari-web]
Title. This is especially useful to categorize the open pull requests based
on their components, so that other folks working in those components can
work on clearing those open pull requests.

Please let me know if you need more clarification on anything discussed
here.

Thanks,
Vivek Ratnavel

On Thu, Jan 4, 2018 at 8:45 AM, Nate Cole <ncole@hortonworks.com<mailto:ncole@hortonworks.com><mailto:ncole@hortonworks.com>>
wrote:

Please also clarify the following scenario:

I’m working on a fix for branch-2.6, and when I’m done, I need to merge to
trunk.

What is the flow?
- Create a fork
- Commit to branch-2.6 (on my fork)
- Commit to trunk (on my fork)
- Create pull request to bring changes to both branches?
Or
- Create a fork
- Commit to branch-2.6 (on my fork)
- Create pull request
- Commit to trunk (on my fork)
- Create pull request

This is exposing my git n00bness





On 1/4/18, 11:32 AM, "Attila Doroszlai" <adoroszlai@hortonworks.com<mailto:adoroszlai@hortonworks.com><mailto:adoroszlai@hortonworks.com>>
wrote:

*   Since this new flow model requires a branch for a commit, we
should enforce a naming strategy. These short-lived feature branches for
commits must be easy to find and remove. We should also make the community
aware that once you have had your pull request merged, you should get rid
of your branch. As for branch naming conventions, I haven't thought through
it very much, but perhaps simply the name of the associated JIRA, such as
AMBARI-12345.

  Correct me if I'm wrong, but the branch to be merged should be created
in your own fork, not in the apache/ambari repo.  Otherwise non-committers
would not be able to create pull requests.  I think this eliminates the
need to coordinate branch naming, although some convention or pattern would
be helpful anyway.

  -Attila

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message