From 1696799d2b2f706df061af4d75a1c50fddf8624e Mon Sep 17 00:00:00 2001 From: Roman Bredehoft Date: Mon, 29 Jan 2024 11:25:29 +0100 Subject: [PATCH] docs: improve contributing section --- docs/developer-guide/contributing.md | 59 ++++++++++++++-------------- 1 file changed, 29 insertions(+), 30 deletions(-) diff --git a/docs/developer-guide/contributing.md b/docs/developer-guide/contributing.md index dec4c7409..adef3e213 100644 --- a/docs/developer-guide/contributing.md +++ b/docs/developer-guide/contributing.md @@ -8,24 +8,22 @@ There are three ways to contribute to Concrete ML: ## 1. Setting up the project -First, you need to properly set up the project by following the steps provided [here](project_setup.md). +First, you need to [fork](https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/working-with-forks/fork-a-repo) the [Concrete ML](https://github.com/zama-ai/concrete-ml) repository and properly set up the project by following the steps provided [here](project_setup.md). ## 2. Creating a new branch -To create your branch, you have to use the issue ID somewhere in the branch name: +When creating your branch, make sure the name follows the expected format : ```shell -git checkout -b {feat|fix|refactor|test|benchmark|doc|style|chore}/short-description_$issue_id -git checkout -b short-description_$issue_id -git checkout -b $issue_id_short-description +git checkout -b {feat|fix|docs|chore}/short_description_$(issue_id) +git checkout -b {feat|fix|docs|chore}/$(issue_id)_short_description ``` For example: ```shell -git checkout -b feat/explicit-tlu_11 -git checkout -b tracing_indexing_42 -git checkout -b 42_tracing_indexing +git checkout -b feat/add_avgpool_operator_470 +git checkout -b feat/470_add_avgpool_operator ``` ## 3. Before committing @@ -38,7 +36,7 @@ Each commit to Concrete ML should conform to the standards of the project. You c make conformance ``` -Conformance can be checked using the following command: +Additionally, you will need to make sure that the following command does not return any error (`pcc`: pre-commit checks): ```shell make pcc @@ -46,7 +44,8 @@ make pcc ### 3.2 Testing -Your code must be well documented, containing tests and not breaking other tests: +Your code must be well documented, provide extensive tests if any feature has been added and must not break other tests. +To execute all tests, please run the following command. Be aware that running all tests can take up to an hour. ```shell make pytest @@ -54,13 +53,13 @@ make pytest You need to make sure you get 100% code coverage. The `make pytest` command checks that by default and will fail with a coverage report at the end should some lines of your code not be executed during testing. -If your coverage is below 100%, you should write more tests and then create the pull request. If you ignore this warning and create the PR, GitHub actions will fail and your PR will not be merged. +If your coverage is below 100%, you should write more tests and then create the pull request. If you ignore this warning and create the PR, checks will fail and your PR will not be merged. There may be cases where covering your code is not possible (an exception that cannot be triggered in normal execution circumstances). In those cases, you may be allowed to disable coverage for some specific lines. This should be the exception rather than the rule, and reviewers will ask why some lines are not covered. If it appears they can be covered, then the PR won't be accepted in that state. ## 4. Committing -Concrete ML uses a consistent commit naming scheme, and you are expected to follow it as well (the CI will make sure you do). The accepted format can be printed to your terminal by running: +Concrete ML uses a consistent commit naming scheme and you are expected to follow it as well. The accepted format can be printed to your terminal by running: ```shell make show_scope @@ -69,39 +68,39 @@ make show_scope For example: ```shell -git commit -m "feat: implement bounds checking" -git commit -m "feat(debugging): add an helper function to draw intermediate representation" -git commit -m "fix(tracing): fix a bug that crashed PyTorch tracer" +git commit -m "feat: support AVGPool2d operator" +git commit -m "fix: fix AVGPool2d operator" ``` -Just a reminder that commit messages are checked in the conformance step and are rejected if they don't follow the rules. To learn more about conventional commits, check [this](https://www.conventionalcommits.org/en/v1.0.0/) page. +Just a reminder that commit messages are checked in the conformance step and are rejected if they don't follow the rules. To learn more about conventional commits, check [this page](https://www.conventionalcommits.org/en/v1.0.0/). ## 5. Rebasing -You should rebase on top of the `main` branch before you create your pull request. Merge commits are not allowed, so rebasing on `main` before pushing gives you the best chance of to avoid rewriting parts of your PR later if conflicts arise with other PRs being merged. After you commit changes to your new branch, you can use the following commands to rebase: +You should rebase on top of the repository's `main` branch before you create your pull request. Merge commits are not allowed, so rebasing on `main` before pushing gives you the best chance of to avoid rewriting parts of your PR later if conflicts arise with other PRs being merged. After you commit changes to your forked repository, you can use the following commands to rebase your main branch with Concrete ML's one: ```shell -# fetch the list of active remote branches -git fetch --all --prune +# Add the Concrete ML repository as remote, named "upstream" +git remote add upstream git@github.com:zama-ai/concrete-ml.git -# checkout to main -git checkout main - -# pull the latest changes to main (--ff-only is there to prevent accidental commits to main) -git pull --ff-only +# Fetch all last branches and changes from Concrete ML +git fetch upstream -# checkout back to your branch -git checkout $YOUR_BRANCH +# Checkout to your local main branch +git checkout main -# rebase on top of main branch -git rebase main +# Rebase on top of main +git rebase upstream/main # If there are conflicts during the rebase, resolve them # and continue the rebase with the following command git rebase --continue -# push the latest version of the local branch to remote -git push --force +# Push the latest version of your local main to your remote forked repository +git push --force origin main ``` You can learn more about rebasing [here](https://git-scm.com/docs/git-rebase). + +## 6. Open a pull-request + +You can now open a pull-request [in the Concrete ML repository](https://github.com/zama-ai/concrete-ml/pulls). For more details on how to do so from a forked repository, please read GitHub's [official documentation](https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/proposing-changes-to-your-work-with-pull-requests/creating-a-pull-request-from-a-fork) on the subject.