-
Notifications
You must be signed in to change notification settings - Fork 592
Contributing Code
This page describes the steps and setup needed to be able to contribute code changes to any of the project repositories.
Essentially there are two different Integrated Development Environments (IDEs) used within the project, IntelliJ and Visual Studio Code (VSCode).
- 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.
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)
Once your PR has been approved and passed all CI checks, use the ready2merge label to indicate the maintainers can merge it.
You must sign-off all commits on the originating branch for a PR. There is a CI check that will fail if any commit in your branch has an unsigned commit or an invalid signature. Git has a built-in flag for sign-off.
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.
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.
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.
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.
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.
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 |
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.
Magma Website • Docs • Code • Contributing • Wiki