Versions Compared

Old Version 2

changes.mady.by.user Gunnar Tapper

Saved on

compared with

New Version 3

changes.mady.by.user Gunnar Tapper

Saved on

Key

  • This line was added.
  • This line was removed.
  • Formatting was changed.

...

Table of Contents

...

Initial Set-Up

Step 1: Configure git E

...

-Mail

Configure git to use your Apache e-mail address.

Code Block
languagebash
git config --global user.email myusername@apache.org

Step 2:

...

Check trafodion-contributors Group

Check that your GitHub user is a public member in the trafodion-contributors group. This allows some permissions with the Jenkins test server.

https://github.com/orgs/trafodion-contributors/people

Step 3: Set Up Work Space

Set up your work space so that you can merge pull requests.

 

Do the following:

  • In VNC/Gnome environment, either add to .bashrc or type at your current shell:

    Code Block
    languagebash
    unset SSH_ASKPASS


  • Pushing code to the Apache repository requires password authentication.
  • Ensure that your work space is cloned from GitHub:
    Code Block
    languagebash
    git clone https://github.com/apache/incubator-trafodion
     

     

     
  • Ensure that you have a remote pointing to the Apache repository. (Setting only the push URL with user name does not seem to work.)
    Code Block
    languagebash
    git remote add apache https://USERNAME@git-wip-us.apache.org/repos/asf/incubator-trafodion.git
     

     

     

Automated Testing

 

You can interact with Jenkins testing via pull request (PR) comments. All of these commands should start with "jenkins," to not confuse other users. You can add more to the end of the message if you want. Jenkins just pattern matches the string, and will ignore trailing comments.

  • If an unknown user submits a PR, then Jenkins automation will post a message to GitHub asking if it is okay to test.
    • Review the pull request. If the code is not malicious and is okay to test, post a comment jenkins, ok
  • If the author is a trusted contributor, you can add them to a white-list of known users.
    • Post a comment jenkins, add user
  • Consider inviting them to the trafodion-contributors GitHub group as well.
    • New commits to the PR will trigger a new build. You can also trigger a retest without a new commit.
    • Post a comment jenkins, retest

Validate Review Critieria

 

The project committee (PPMC) has agreed that the following review criteria are used for contributions:

  • Code Review(s)
  • Time available for comments
  • Testing
    • Be sure that you wait for all pending tests!
    • New commits, even those that are just merging with latest master branch, trigger new test run.
  • Legal
  • Other

 

Guidelines for Code Reviews

 

Anyone can be a reviewer: participating in the review process is a great way to learn about the Trafodion development processes.

 

Some things are necessary to keep in mind when doing code reviews:

 

  1. C code should comply with the [[cplusplus-guidelines,C Guidelines]].

  2. The code shouldlook like the code around it, to make the code more uniform and easier to read.

  3. New test cases should be implemented as described in Modifying Tests.

 

General Flow

 

Review is a conversation that works best when it flows back and forth. Submitters need to be responsive to questions asked in comments, even if the score is +0 from the reviewer. Likewise, reviewers should not use a negative score to elicit a response if they are not sure the patch should be changed before merging.

 

For example, if there is a patch submitted which a reviewer cannot fully understand because there are changes that aren’t documented in the commit message or code documentation, this is a good time to issue a negative score. Patches need to be clear in their commit message and documentation.

 

In almost all cases, a negative review should be accompanied by clear instructions for the submitter how they might fix the patch.

 

There may be more specific items to be aware of inside the projects' documentation for contributors.

 

Contributors may notice a review that has several +1’s from other reviewers, passes the functional tests, etc. but the code still has not been merged. As only committers can approve code for merging, you can help things along by getting a committers’s attention in dev@trafodion.incubator.apache.org and letting them know there is a changeset with lots of positive reviews and needs final approval.

 

Be Aware of Licensing Concerns

Trafodion is developed and provided using the Apache 2.0 license. In order to not compromise that license, we must be careful about accepting code contributions that are not original code. We cannot accept code forked from other open source projects that have a non-permissive license for example.

 

If a reviewer suspects that contributed code is not original without attribution, the reviewer should -1 the change, and in the review comment, state the concern about originality and licensing. If the contributed code is proven to be unoriginal and with an incompatible license, the contribution should be rejected. If this happens, it is recommended that the Trafodion PPMC privately discuss with the contributor the importance of proper licensing and proper attribution or originality. The importance of attribution, originality, and licensing is often difficult for some new contributors, for some students, and for some people from various cultures of origin, and this should be seen as a learning and teaching opportunity.

Merge Pull Requests

Step 1: Check Status

Check the pull request status on GitHub, at the bottom of the pull request page. It will tell you if there are any merge conflicts with master branch.

Info

If there are conflicts, either ask the contributor to merge up, or be prepared to resolve the conflicts yourself.

Step 2: Create Local Merge Branch

Create a local merge branch, based on the latest, greatest.

Code Block
languagebash
# You will be prompted for your Apache password
git fetch apache
git checkout -b mrg_12345 apache/master

Step 3: Fetch Pull Request Branch

Fetch pull request branch to default destination FETCH_HEAD.

Code Block
languagebash
git fetch origin +refs/pull/12345/head

Step 4: Merge Locally

Merge locally, giving message that includes JIRA ID.

Code Block
languagebash
git merge --no-ff -m "Merge [TRAFODION-XYZ] PR-12345 Whizbang feature" FETCH_HEAD

NOTES

  • Sometimes you might want to squash their branch into a single commit. If so, add the --squash option.
  • If you forget the -m option, you end up with a less than helpful default comment.

    • Before you push the commit, you can fix the comment by:

      Code Block
      languagebash
      git commit --amend

Step 5: Additional Checks

Additional checks of what you are preparing to push.

Code Block
languagebash
git log apache/master..HEAD
git diff apache/master HE

Step 6: Push Changes

Push changes to the Apache repository, specifying the source and the destination branch.

Code Block
languagebash
# You will be prompted for your Apache password
git push apache HEAD:master`

Completion

 

  1. Close Jira, if appropriate, or ask the contributor to do so.
  2. If ASF automation does not close the pull request, ask the contributor to do so.