Skip to content

Guidance for contributors

Jump to bottom Edit New page
Roie Danino edited this page Oct 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 PR with Merge Commits in Git History

Let's say we have a PR that conflicts with master and we've merged with master in order to resolve, then, we've added a few more commits. Now, the PR has been approved and we need to squash our commits, we have some commits coming from the merge we did earlier in our git history making it a non-trivial squash. The git tree would probably look like this:

  graph TD;
      A-->B;
      Merge_Parent_From_Master-->Merge;
      B-->Merge;
      Merge-->C;
      C-->D;
Loading

What we want is to have a single commit containing all of our changes as a child of Merge_Parent_From_Master:

  graph TD;
      Merge_Parent_From_Master-->id2([A with changes from B,C and D squashed]);
Loading
  1. git reset --hard <A - first commit in PR>
  2. git checkout <D - last commit in branch> -- .
  3. git status # should show all the files that have been modified between your first commit A and last commit D
  4. git commit --amend # Add the changes to A
  5. Find the parent of the merge commit, you can use: git log --graph --oneline <D - last commit>
  6. git rebase <parent of the merge commit> # commit A should be the child of the merge commit parent
  7. git checkout <D - last commit> -- .
  8. git log --graph --oneline # should show A as the child of the merge commit

More details here:

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:

UCX - best communication library ever

Home
Bi-Weekly conference call
Guidance for contributors
Code style checking
UCF Collectives WG

Clone this wiki locally