Skip to content

Guidance for contributors

Ilia Yastrebov edited this page Dec 4, 2023 · 45 revisions

UCX uses Git for source code management, Azure CI for continuous integration, and Google’s C++ test framework for unit testing. Before you make any contributions to the projects, please sign a copy of our CLA https://openucx.org/license/ and email the CLA to admin at ucfconsortium.org

General guidelines

  1. Follow the code style and logging style.

  2. Use automatic code style checking to format your code.

  3. Format the commit message as follows:
    MODULE/UNIT1/UNIT2/..: Summary
    for example:
    UCT/SHM: Fix shared memory deadlock.

  4. Everything should be submitted as a pull request ("PR") from a branch.
    NO DIRECT PUSH TO MASTER!

  5. No PRs over 500 lines of added code.
    3rd party code can be larger, but has to be in a separate PR

  6. API changes have to be in a separate PR, which includes only the minimal set of changes required for making the code compile and tests pass. The implementation of new functionality should be a stub.
    API changes have to be approved by bbenton. API changes should including the motivation of the change and a note if it does/doesn't break ABI/API

  7. When both renaming and changing a file, separate the two to different commits.

  8. If you need to fix your PR, there are 2 options: amend (replace) your commit, or add new commit with fixes.

  • Before anyone has posted comments on the PR, it's allowed to amend.
    Example: Fixing bugs found in automatic tests.
  • If some comments have been posted, the fixes should be in a new commit.
    Reason: Tracking fixes of code review.

Git Workflow to Submit a Feature or Bug Fix

  1. Fork the project from openucx/ucx to create your copy gituser/ucx

  2. Clone the code from gituser/ucx: git clone <XXXX>

  3. Create a new branch for your bugfix, feature, or enhancement: git branch topic/bugfix_123

  4. Switch your workspace to a newly created branch: git checkout topic/bugfix_123

  5. Commit your changes: git commit .

  6. Push the changes in your branch (topic/bugfix_123) to your copy gituser/ucx : git push origin topic/bugfix_123. If the default upstream for the branch is not defined, add -u flag to the push command. The flag sets default upstream for the branch.

  7. Submit your change to the openucx/ucx by opening a pull request. After submitting the PR, the code is reviewed by the reviewers/maintainers. Once the reviewers and authors reach an agreement, the author requests the maintainer to merge the changes.

  8. Delete the branch: git push origin :topic/bugfix_123

Squashing a PR with merge commits in history

Consider a branch that had conflicts with master, which were resolved by merging from it, and then more commits were added. As the git history contains some commits from that merge, its squash will be a non-trivial one. The git tree will look somewhat like:

  graph TD;
      A-->B;
      parent_master-->merge;
      B-->merge;
      merge-->C;
      C-->D;
Loading

The goal is to have a single commit containing the whole PR changes, as a child of parent_master:

  graph TD;
      parent_master-->id2([A with changes from B,C and D squashed]);
Loading

After PR is approved either use script below:

  1. ./contrib/squash_commit.sh -m "Commit message" -r upstream/master
  2. git push -f

Or use the steps:

  1. backup=$(git rev-parse HEAD)
  2. Find the parent of the most recent merge commit, you can use: git log --graph --oneline <D>
  3. git reset --hard # making sure checkout matches approved PR
  4. git clean -fdx
  5. git reset <parent_master>
  6. git add --all # also add newly added files
  7. git commit
  8. git diff HEAD..origin/pr_name # should be empty
  9. Finally run git push -f
  10. In case something goes wrong: git reset --hard $backup. Also, git reflog might provide some insights.

See also:

Code reviews

  1. One or more of UCX maintainers has to approve (:+1:) the PR in order to merge it.

  2. Before requesting a review, please make sure the automatic tests pass.
    See the UCX Testing Procedure

  3. PRs which are waiting for review should be marked with the label "Ready for review". It's also advisable to tag the relevant reviewer.

  4. PRs which were reviewed and currently waiting for a fix and/or response, are marked with "Waiting for Author Response".

Reviewers

Everybody are welcome to post comments on every PR. However a final approval should be given by the maintainers. Also, changes which affect API (especially UCP API) should be coordinated with UCX users and approved by all maintainers.

List of maintainers:

Primary reviewers per component:

Clone this wiki locally