Versions Compared

Old Version 5

changes.mady.by.user Gunnar Tapper

Saved on

compared with

New Version Current

changes.mady.by.user Sean Broeder

Saved on

Key

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


Excerpt

Provides information describing how a committer reviews and approves pull requests.


...

Table of Contents
maxLevel4
indent20px

...

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

Alternatively, you can edit your .gitconfig file so that you don't need to enter the commands interactively by adding

[user]
name = myusername
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

If you are not listed in the above group, send a message to one of the members listed as an owner and request to be added to the group.

Step 3: Set Up Work Space

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

...

  • 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. This means that fetching the apache remote prompts for a password, but it is ignored. You can hit return without entering password, and fetch still works! You must enter password only when pushing to apache remote.)

    Code Block
    languagebash
    git remote add apache https://USERNAME@git-wip-usUSERNAME@gitbox.apache.org/repos/asf/incubator-trafodion.git


Automated Testing

...

  • 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.
  • New commits to the PR will trigger a new build. You can also trigger a retest without a new commit.
    • Post a comment comment: jenkins, retest

...

  • For extensive or risky changes, you may want to run a fuller set of tests.
    • Post a comment: jenkins, extra test
    • This runs additional jobs, running several suites o CDH and HDP distros, as well as full phoenix suite.

Validate Review Criteria

 

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 

...

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 should look 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.

...

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 - The Easy Way

The easy way is to the click the big green button on github that says "Merge Pull Request".

However, in order for committers to have that write-access via github, they must have your apache ID linked to their github ID.  Follow directions at: https://gitbox.apache.org/setup/

Merge Pull Requests - The Hard Way 


Note
titleTarget Branch

Before beginning, make careful note of what branch you are merging to. At the top of the pull request, you'll see words like, "So-and-so wants to merge n commits into apache:target_branch from github_user:some_other_branch. Most of the time, target_branch will be "master", but occasionally you'll see a release branch instead, e.g., "release1.3". In the instructions below, if the target_branch is not "master", replace "master" with the name of the target branch.

 

 

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.

...

Code Block
languagebash
# You will be prompted for your Apache passwordpassword, but you do not have to enter it for fetch. Just hit return.
git fetch apache
git checkout -b mrg_12345 apache/master

...

Code Block
languagebash
git merge --no-ff -m "Merge [TRAFODION-XYZ] PR-12345 Whizbang feature" FETCH_HEAD
 OR
git merge --squash FETCH_HEAD
git commit  # edit comment, give good headline and detailed body
            # Also include phrase "This closes #<PR num>" substituting in the pull-request number

NOTES

  • Sometimes for changes with many commits, you might want to squash their branch into a single commit. If so, add the --squash option.
    • On the plus side, squashing a developer branch into single commit puts all the file related changes into a single commit. Easier to find, back-port, etc.
    • On the minus side, squashing breaks the change from the developer history. Be extra careful to edit the comment to clearly identify the developer and combined commit comments. 
  • 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


...

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

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 

...

:master

Publish Web Site Updates

There are three main types of content on the Apache Trafodion web site:

  • The main site content
  • Trafodion documents, generated from the Trafodion source code
  • Trafodion apidocs, generated from the Trafodion source code and built objects

Depending on which type of content you want to update, you will need to include different steps of the instructions shown below, required steps are marked with an X:

StepmaindocsapidocsDescriptionCommand
1XXXGet the Trafodion source code from gitsee "Download and Install Source"
2 XXCheck out a specific release X.Y.Z, if you want to update the documentation for a previous release
git checkout rel/X.Y.Z
3XX Build Trafodion site & documentation, after modifications to the documents, see Modifying Documents.
cd trafodion
source env.sh
mvn post-site 
4  XBuild Trafodionsee "Build Source"
5XXXGet the web site content from git
git clone https://ApacheUserName@gitbox.apache.org/repos/asf/trafodion-site.git
cd trafodion-site
git checkout -b asf-site origin/asf-site
6XX Copy built web/doc changes to main site. This should be done from the master branch (release branch while a release is in progress) ONLY. This should not over-write release-specific docs.

see "Modify Web Site"

cp -R trafodion/docs/target/*   trafodion-site/
7 X Copy the release-specific built documentation. This is for non-master branches.
cp -R trafodion/docs/target/docs/<release>   trafodion-site/docs/
8  XBuild apidocs (note: X.Y.Z is the release number, see step 2 above). TBD: Instructions for copying apidocs to site repository.
cd trafodion-site/apidocs
build_apidocs.sh -o X.Y.Z
9XXXCommit changes in the web site repository
git status
git add --all
git commit 
10XXXPush changes back to ASF repository
git push origin HEAD:asf-site
11XXXCheck that the website, https://trafodion.apache.org is refreshed. If it isn't, make a small white-space change to the index.html file to trigger the Apache gitpubsub that does the update. 
12X  Once published, run https://validator.w3.org against updated pages to ensure that there are no broken links 
13XXXClose the JIRA and/or pull request, if applicable 

...