|
|
|
# How to create & review a merge request?
|
|
|
|
|
|
|
|
Everybody. Only comitters will be allowed to push on the project and launch pipelines but nothing stops you from contribute & review other's code.
|
|
|
|
|
|
|
|
This page is here to guide you in the process of creating a clean merge request or review a merge request.
|
|
|
|
|
|
|
|
[[_TOC_]]
|
|
|
|
|
|
|
|
# How to review a merge request
|
|
|
|
|
|
|
|
#### :new:First contribution ?
|
|
|
|
|
|
|
|
If this is the first contribution of a new contributor, redirect him to the contribution rules before reviewing code.
|
|
|
|
|
|
|
|
#### :deciduous_tree:Git flow
|
|
|
|
|
|
|
|
##### MR configuration
|
|
|
|
|
|
|
|
###### MR to `main` is forbidden
|
|
|
|
|
|
|
|
Nothing must be merged to the `main` branch. Only people creating release are allowed to merge to `main`.
|
|
|
|
|
|
|
|
###### MR name
|
|
|
|
|
|
|
|
Must be explicit & fitting ([following commit convention](https://gitlab.eclipse.org/groups/eclipse/aidge/-/wikis/Get-good-at-git#how-to-write-a-commit-message-)), this is especially important in case the [MR is squashed](https://docs.gitlab.com/ee/user/project/merge_requests/squash_and_merge.html#set-default-squash-options-for-a-merge-request),as the MR name will become the commit name by default.
|
|
|
|
|
|
|
|
If the MR is a fix, it must be labelled fix. To see what can be labelled fix, head to [hwo to write a commit message](https://gitlab.eclipse.org/groups/eclipse/aidge/-/wikis/Get-good-at-git#how-to-write-a-commit-message-)
|
|
|
|
|
|
|
|
##### MR must be linked to an issue
|
|
|
|
|
|
|
|
This issue will provide CONTEXT to the modifications provided : do not only explain WHAT but also WHY.
|
|
|
|
|
|
|
|
##### MR description
|
|
|
|
|
|
|
|
Must mention the linked issue. Can also contain additional context specific to this MR.
|
|
|
|
|
|
|
|
###### Related MR
|
|
|
|
|
|
|
|
If the MR is related to other merge requests they must be linked via an issue. To do that simply mention the issue in the MR description.
|
|
|
|
|
|
|
|
###### MR dependencies
|
|
|
|
|
|
|
|
If a merge request(A) depends on another merge request(B) (i.e. A must be merged after B), [B must be marked as a dependency of A](https://docs.gitlab.com/ee/user/project/merge_requests/dependencies.html#create-a-new-dependent-merge-request)
|
|
|
|
|
|
|
|
##### Commit history
|
|
|
|
|
|
|
|
[must follow the contribution guidelines](https://gitlab.eclipse.org/groups/eclipse/aidge/-/wikis/Contributing#commit-recommendations)
|
|
|
|
|
|
|
|
#### 🧪 Testing
|
|
|
|
|
|
|
|
Every new feature / bug fix must be accompanied by a new test that ensures the stability of the codebase (i.e. that the error won't happen anymore).
|
|
|
|
|
|
|
|
Proofread and verify tests.
|
|
|
|
|
|
|
|
#### :train: CI Pipeline
|
|
|
|
|
|
|
|
> :warning:**If you are a comitter**
|
|
|
|
>
|
|
|
|
> Triple check that the person has not modified the `.gitlab` files before running any pipeline as this could lead to having someone running hazardous code on our servers !!
|
|
|
|
|
|
|
|
CI pipeline must pass.
|
|
|
|
|
|
|
|
#### Aidge bundle related MR
|
|
|
|
|
|
|
|
If the merge request is not a simple an operator addition it must be accompanied by an Aidge bundle merge request where sub-modules hashes are updated accordingly to ensure that the modifications do not break anything else in the codebase.
|
|
|
|
|
|
|
|
#### :book: Documentation
|
|
|
|
|
|
|
|
Each function must be provided with :
|
|
|
|
|
|
|
|
* doxygen for C++
|
|
|
|
* docstring for python/pybind
|
|
|
|
|
|
|
|
#### :interrobang: Special Merge requests
|
|
|
|
|
|
|
|
##### New operator
|
|
|
|
|
|
|
|
When merging an MR for the support of a new operator update the Epic: https://gitlab.eclipse.org/groups/eclipse/aidge/-/epics/2
|
|
|
|
|
|
|
|
If an equivalent onnx operator is available, the reviewer must understand how it works.
|
|
|
|
|
|
|
|
> :warning: **The onnx operator is a reference and not a golden rule**
|
|
|
|
>
|
|
|
|
> The real specification is that the operator must be importable & exportable with regard to onnx over all available opsets.
|
|
|
|
|
|
|
|
Required :
|
|
|
|
|
|
|
|
* Python bindings of the operator
|
|
|
|
* Tests on core, backends & onnx repos.
|
|
|
|
|
|
|
|
##### API
|
|
|
|
|
|
|
|
Change or addition, must update the corresponding documentation part.
|
|
|
|
|
|
|
|
If the reviewer detect changes provided to a core functionnality that affects the behavior of existing objects then it should call a referee ( @cmoineau @pineapple or @olivierbichler) to approve changes.
|
|
|
|
|
|
|
|
##### Gitlab file
|
|
|
|
|
|
|
|
Each modification of gitlab CI pipeline must be thoroughy reviewed as these modifications will affect all repos.
|
|
|
|
|
|
|
|
The referee for these merge requests is @gregkub. |
|
|
\ No newline at end of file |
