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.

• How to create & review a merge request?
• How to review a merge request
  • 🆕 First contribution ?
  • 🌳 Git flow
    • Merge Request configuration
      • Merging to main is forbidden
      • Merge request name
      • It must be linked to an issue
      • Description
      • Related MR
      • Dependencies
    • Commit history
  • 🧪 Testing
  • 🚋 CI Pipeline
  • 🧰 Aidge bundle related MR
  • 📖 Documentation
  • ⁉️ Special Merge requests
    • New operator
    • API
    • Gitlab file

How to review a merge request

🆕 First contribution ?

If this is the first contribution of a new contributor, redirect him to the contribution rules before reviewing code.

🌳 Git flow

Merge Request configuration

Merging to main is forbidden

Nothing must be merged to the main branch. Only people creating release are allowed to merge to main.

Merge request name

Must be explicit & fitting (following commit convention), this is especially important in case the MR is squashed,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

It must be linked to an issue

The Merge request must be linked to an issue that will provide CONTEXT to the modifications provided : do not only explain WHAT but also WHY.

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.

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

Commit history

must follow the contribution guidelines

🧪 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.

🚋 CI Pipeline

⚠️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.

📖 Documentation

Each function must be provided with :

• doxygen for C++
• docstring for python/pybind

⁉️ Special Merge requests

New operator

When merging an MR for the support of a new operator update the Epic: &2

If an equivalent onnx operator is available, the reviewer must understand how it works.

⚠️ 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.