Contributing Code - magma/magma Wiki
This page describes the steps and setup needed to be able to contribute code changes to any of the project repositories.
- IntelliJ works well with the virtual machine setup.
- VSCode works well with the devcontainer setup. You can use devcontainer locally as well as in the cloud with GitHub Codespaces.
Where to start?
You can use the
good first issue tag on our GitHub repository to search for good starter tasks. As you are working on your starter task, refer to the subsections of this page for guidance on how to author a pull request (PR) and Code Contributing Conventions for code conventions in Magma. Once your PR is approved and merged, you're now an official contributor to the Magma project!
Also join our community Slack channel via the link from the Community page and say hello in the
#general channel. We can point you toward a good starter task or guide you choosing appropriate tasks given your skill set and the Magma roadmap.
Next to the GitHub Wiki also consider the documentation in DocuSaurus as a valuable information source.
As coding guidelines are quite extensive, you can find them in the separate page about Code Contributing Conventions.
Magma follows the standard "fork and pull-request" development workflow. For more information on this workflow, consider the following resources:
- GitHub Documentation
- How To: Fork a GitHub Repository & Submit a Pull Request
- GitHub Standard Fork & Pull Request Workflow (text only version)
Commit and pull-request guidelines
You must sign-off all commits on the originating branch for a PR, which certifies that you wrote it or otherwise have the right to pass it on as an open-source contribution. The rules are pretty simple: if you can certify the below (from developercertificate.org):
Developer's Certificate of Origin 1.1
By making a contribution to this project, I certify that:
(a) The contribution was created in whole or in part by me and I have the right to submit it under the open source license indicated in the file; or
(b) The contribution is based upon previous work that, to the best of my knowledge, is covered under an appropriate open source license and I have the right under that license to submit that work with modifications, whether created in whole or in part by me, under the same open source license (unless I am permitted to submit under a different license), as indicated in the file; or
(c) The contribution was provided directly to me by some other person who certified (a), (b) or (c) and I have not modified it.
(d) I understand and agree that this project and the contribution are public and that a record of the contribution (including all personal information I submit with it, including my sign-off) is maintained indefinitely and may be redistributed consistent with this project or the open source license(s) involved.
Then you just add a line to every git commit message:
Signed-off-by: Joe Smith [email protected]
You need to use your real name to contribute (sorry, no pseudonyms or anonymous contributions). If you set your user.name and user.email git configs, you can sign your commit automatically with git commit -s.
Pull-request and commit message title are following conventional commits format
- Supported scopes and types can be found in the CI check definition. The list is based on the original one from the Conventional Commits project.
- In short, the format is
type(scope): Title, e.g.
fix(agw): Fix pyroute2 dependency.
Labeled backward-breaking pull requests
Backward-breaking PRs must use the
breaking change label. All breaking changes and their mitigation steps will be aggregated in the subsequent release notes. A breaking change fits one or more of the following criteria:
- The change will require manual intervention on the next upgrade (e.g. a data migration script).
- The change will break the southbound interfaces between AGW and Orchestrator in a way that will require both components to upgrade in a coordinated way.
Convincing test plan
For non-trivial PRs, codeowners will require you to include a test plan detailing any manual verification steps you took.
Imperative mood for pull-request title and description
A simple way to check this is to mentally prepend the phrase "This commit will ..." to the title and each bullet point of your description.
Testing code locally
During their execution, the integration tests change the configuration in the Magma VM. If the tests are not completed successfully, the changes are not removed and the VM may show a different behaviour. In this case, the VM must be reset to restore the original state.
In order to avoid this situation, you should always run the
sudo tests first. Subsequently, the integration tests can be executed. If you swap the execution order, the sudo tests would fail.
Workaround: A VirtualBox snapshot of the Magma VM can be created after provisioning the VM but before running the integration tests.
Code Review Process
If you create a pull request against the Magma master branch, a codeowner will automatically be assigned to the PR for review. In addition, most PRs will be tested by an extensive set of CI checks (see below).
Once all required CI checks passed and you have at least one approval from the respective codeowners, you can set the
ready2merge label and a maintainer will get your changes landed.
Continuous Integration (CI) / Continuous Deployment (CD)
When a PR is submitted to the Magma repository, a suite of CI tests and analysis is executed. Checks marked as required must pass for a PR to be merged.
Most of the checks are implemented as GitHub Actions, which need to run successfully. Anything not in GitHub to date is planned to be migrated in the future. Before adding or updating an action, familiarize yourself with Secure Use of Github Actions.
ToDo: If and when documentation on components is migrated, the links here need to be updated.
Note: No user mentions in GitHub wiki so far: https://github.com/isaacs/github/issues/205.
Merge blocking CI checks are listed below.
|Check Name||Purpose||Owner||Remediation Steps|
|DCO Check||Check PR is signed off||any maintainer||PR guidelines|
|Semantic PR||PR format checker||any maintainer||PR guidelines|
|mergefreeze||Stop merges during a code freeze||@themarwhal||N/A|
|orc8r-build||Validate Orc8r builds||@magma/approvers-orc8r||Orc8r build|
|cloud-test||Run Orc8r unit tests||@magma/approvers-orc8r||Orc8r tests|
|cloud_lint||Check Orc8r changes satisfy golint||@magma/approvers-orc8r||Orc8r tests|
|insync-checkin||Ensure generated files are committed||@magma/approvers-orc8r||Orc8r tests|
|feg-build||Validate FeG builds||@magma/approvers-feg||FeG build|
|feg-precommit||Run FeG unit tests||@magma/approvers-feg||FeG tests|
|nms-build||Validate NMS builds||@magma/approvers-nms||NMS build|
|eslint||Ensure NMS changes satisfy eslint||@magma/approvers-nms||NMS tests|
|nms-flow-test||Validate NMS changes satisfy flow||@magma/approvers-nms||NMS tests|
|nms-yarn-test||Run NMS unit tests||@magma/approvers-nms||NMS tests|
|nms-e2e-test||Run NMS end to end tests||@magma/approvers-nms||NMS tests|
|lte-test||Run AGW Python unit tests||@magma/approvers-agw||AGW tests|
|mme_test||Run MME and sctpd unit tests||@magma/approvers-agw-mme||AGW tests|
|session_manager_test||Run SessionD unit tests||@magma/approvers-agw-sessiond||AGW tests|
|orc8r-gateway-test||Run Golang unit tests for orc8r/gateway||@magma/approvers-agw||AGW tests|
|cwag-precommit||Run CWAG unit tests||@magma/approvers-cwf||CWAG tests|
|Magma-OAI-Jenkins||OAI's MME integration test run on OAI Jenkins||@rdefosse||N/A|
|cwf-operator-build||Validate CWF deployer builds||@magma/approvers-cwf||TODO|
|cwf-operator-precommit||Run CWF deployer unit tests||@magma/approvers-cwf||TODO|
|Python Format Check||Ensure Python changes are formatted||@themarwhal||AGW formatting|
|lint-clang-format||Ensure C++/C changes are formatted||@magma/approvers-agw||AGW formatting|
|markdown-lint||Ensure documentation changes are formatted||@magma/approvers-docs||Docs precommit|
|C/C++ unit tests with Bazel||Run all C/C++ tests covered with Bazel||@magma/approvers-agw||AGW tests|
The CI checks listed below do not block merging on failure.
|Check Name||Purpose||Point of Contact||Remediation Steps|
|Shellcheck by Reviewdog||Annotate PRs with shell script static analysis||@electronjoe||N/A|
|GCC Warnings & Errors / gen_build_container||Passive check generates container for GCC builds||@electronjoe||N/A|
|GCC Warnings & Errors / build_oai||Annotate PRs with any GCC warnings or errors for MME||@electronjoe||N/A|
|GCC Warnings & Errors / build_session_manager||Annotate PRs with any GCC warnings or errors for session_manager||@electronjoe||N/A|
|Jenkins CWAG Libvirt||Run CWF integration tests||@themarwhal @mattymo||TODO|
|C / C++ code coverage||Upload AGW C/C++ code coverage||@electronjoe||N/A|
|Jsonlint gateway.mconfig files||Syntax check for mconfig jsons||@alexzurbonsen||validate json syntax|
|DP Configuration controller unit tests||Run configuration-controller Unit tests||@magma/approvers-domain-proxy||N/A|
|DP Active mode controller unit tests||Run active-mode controller unit tests||@magma/approvers-domain-proxy||N/A|
|DP Radio controller unit tests||Run radio controller unit tests||@magma/approvers-domain-proxy||N/A|
|Domain proxy db migration test||Validate alembic database migrations||@magma/approvers-domain-proxy||Check alembic migrations|
|DP DB Service unit tests||DB service unit tests||@magma/approvers-domain-proxy||N/A|
|Domain proxy integration tests||Run domain-proxy integration tests||@magma/approvers-domain-proxy||N/A|
|DP Helm chart smoke tests||Run domain-proxy hel m chart smoke tests||@magma/approvers-domain-proxy||N/A|
How to add a new CI check to Magma
To add a non-blocking check,
- the table of non-blocking checks must be updated to include the new check with at least the Purpose and PoC filled out.
To add a blocking check,
- the table of blocking checks must be updated to include the new check with all columns filled out.
- File a proposal to outline the motivation and any relevant timelines and assign to a relevant codeowner or TSC (An example proposal)
- Once the proposal is accepted, announce the plan in #dev and coordinate with one of the repo admins to make the switch.