This document outlines the best practice, using git and CI, which must be followed for all contributions to janus-core. Also contained are instructions and tips for managing your fork of the project which will help keep merges clean and avoid many headaches.
In brief the rules for contribution are as follows:
- Follow the branch, fix, merge model, from your own fork or fork/branch model.
- An issue must be created for every piece of work (bug, feature, etc.)
- Pull/merge requests will not be accepted without a review
- New features must have a test
- All tests must pass, no regressions may be merged
- Open an issue for each piece of work done.
- Open issues for design discussions. The answers may be important for newer members.
- When adding features, e.g. a new MLIP, use comments to provide succinct reports on progress.
Labels may be assigned to issues to help classify them. Examples include:
- Bug: something is not working as expected. We typically aim to fix these ASAP
- Enhancement: adding or improving on features
- Testing: adding or enhancing unit testing or CI
- Documentation: adding or enhancing documentation
- Question: Something is unclear
All merge requests will be reviewed to ensure the integrity of the code.
The reviewer(s) have the following responsibilities:
- Ensuring all contribution rules have been followed
- Ensuring the coding style is adhered to
- Only accepting a merge if all tests have passed
- Using the comments system to request changes for the submitter to make
GitHub Actions will automatically run a pre-commit and enforce the coding style. To reduce the number of CI failures, please run the pre-commit locally before you push to your repository.
pre-commit run --all-files
The GitHub instance hosts an upstream repository, which we will refer to as upstream. Contributors will work on their own personal copies of the repository by creating forks. This allows us to keep upstream clean (one commit per merge request, if possible, otherwise commits should be squashed before merging. All commits should pass tests.) while users may work on their own fork, creating commits and changing the code as they see fit. Only in exceptional circumstances are branches allowed in upstream. Please discuss in the zulip channel before you push any.
The upstream repository may be cloned as follows,
git clone git@github.com:stfc/janus-core.git janus-core-stfc
A fork is created using the web UI. It may then be cloned for a user called 'username' as follows,
git clone git@github.com:username/janus-core.git janus-core-username
All work should follow the workflow of branch, fix, merge. Let us assume you have an issue with your code which needs to be fixed.
Create a new branch for the issue on the dashboard of your fork, we will assume the branch is called 'issueXYZ'. Clone the branch,
$ git clone -b issueXYZ --single-branch git@github.com:username/janus-core.git janus-issueXYZ
Alternatively you can create the branch in the cli using
# clone the repository, if you already have a local repository this is not nessecary
$ git clone git@github.com:username/janus-core.git janus-issueXYZ
$ pushd janus-issueXYZ
# create and checkout a new branch (this is equivilent to git branch followed by git checkout)
$ git checkout -b issueXYZ
# create a remote tracking branch for you to push your changes to
$ git push -u origin issueXYZ
Fix whatever is wrong. Use git status
to see which files you have
changed and prepare a commit.
# stage changes
$ git add <filename|folder> # to add the new things
# commit the changes with a clear and brief message
$ git commit -m "<commit message>"
# push the commit to origin
$ git push
Follow the provided link after the push and continue to the web interface. Add any relevant labels or milestones and assign a reviewer. Compare the code and if you are happy click to submit your pull request.
After the pull request has been submitted, tests will be run and your reviewer will be notified.
If all is OK with the commit your reviewer may set the request to be merged once all tests pass. Otherwise the reviewer may open discussions using the GitHub comment system to point out issues that may need to be addressed before the commit can be merged.
If changes need to be made you may make more commits onto your branch. When you push your branch to your fork the pull request will be automatically updated to use the latest commit. Reply to the discussions to indicate when and how they have been addressed.
If your branch has become out of sync with upstream then conflicts may arise. Sometimes these cannot be automatically resolved and you will need to resolve them by hand. GitHub provides instructions for this, or you can follow this routine,
# add upstream as a remote if you have not already
$ git remote add upstream git@github.com:stfc/janus-core.git
# get the changes to upstream since you started working on your issue
$ git fetch upstream
# merge these changes into your branch (assuming you want to merge into the main branch on upstream)
$ git merge upstream/main
# resolve any conflicts
# push to your fork
$ git push
Alternatively you may use rebase which will replay the changes you made in your branch on top of upstream/main. However, be sure you understand the differences between merge and rebase
# add upstream as a remote if you have not already
$ git remote add upstream git@github.com:stfc/janus-core.git
# get the changes to upstream since you started working on your issue
$ git fetch upstream
# merge these changes into your branch (assuming you want to merge into the main branch on upstream)
$ git rebase -i upstream/main
# resolve any conflicts
# push to your fork
$ git push
By adding two remotes, one for upstream and one for your fork it is possible to keep your fork in sync with upstream. This will greatly simplify merge requests. GitHub also offers a sync functionality in their web UI that achieves the same.
# clone your fork
$ git clone git@github.com:username/janus-core.git janus-core-username
pushd janus-core-username
# add a remote for upstream
$ git remote add upstream git@github.com:stfc/janus-core.git
These commands need to be done only once. git remote -v
shall show you
the origin and project fetch and push links
$ git remote -v
origin git@github.com:username/janus-core.git (fetch)
origin git@github.com:username/janus-core.git (push)
upstream git@github.com:stfc/janus-core.git (fetch)
upstream git@github.com:stfc/janus-core.git (push)
When you need to sync your fork with upstream, do the following,
# get the latest commits from upstream
$ git fetch upstream
# ensure you are in the main branch of your fork
$ git checkout main
# merge your main branch into the main branch of upstream
$ git merge upstream/main
# push these changes back to the remote of your fork (origin)
$ git push
of course one can use a similar process to merge any other branch or available projects.
When working on an issue you may use multiple commits. When you are ready to create a merge request, you should squash your changes into one commit in order to keep upstream clean. This is most easily achieved with an interactive rebase.
Assuming you have made five commits,
# rebase your branch five commits before HEAD i.e. where your branch originally diverged
$ git rebase -i HEAD~5
# follow the instructions. 'pick' the first commit then 'sqaush' or 'fixup' the rest.
# You should now be left with a single commit containing all your changes
# Push your commmit to the remote, use --force-with-lease if you have already pushed this branch to
# 'rewrite history'
$ git push origin branchname --force-with-lease
using force is a powerful and dangerous option, use it only if you know 150% nobody else touched that branch.
Deleting branches from the web interface will get rid of the remotes and not of your local copies. The local branches left behind are called stale branches. To get rid of them
$ git remote prune origin
To delete a local branch
$ git branch -d localBranch
if unmerged commits exists but you still want to delete use
$ git branch -D localBranch
To delete a remote branch on the remote origin use
$ git push -d origin remoteBranch
Ensure that any code you add does not reduce the code coverage in a meaningful way. Reviewers may insist on new test to be added, please cooperate.