DOC: Add a copy of the software dev document from hi-ml

docs/source/index.rst

@@ -28,7 +28,7 @@ InnerEye-DeepLearning Documentation
    :maxdepth: 1
    :caption: Further reading for contributors
 
-   md/pull_requests.md
+   md/software_design_overview.md
    md/testing.md
    md/contributing.md
    md/deploy_on_aml.md

docs/source/md/contributing.md

@@ -1,20 +1,27 @@
 # Contributing
 
-This document describes guidelines for contributing to the InnerEye-DeepLearning repo.
+This document contains detailed guidelines for contributing to the InnerEye-DeepLearning repo.
+
+For the overarching software development process, refer to the [process description](software_development_process.md).
 
 ## Submitting pull requests
 
-- DO submit all changes via pull requests (PRs). They will be reviewed and potentially be merged by maintainers after a peer review that includes at least one of the team members.
+- DO submit all changes via pull requests (PRs). They will be reviewed and potentially be merged by maintainers after
+  a peer review that includes at least one of the team members.
+- DO NOT mix independent and unrelated changes in one PR. PRs should implement one change, and hence be small. If
+  in doubt, err on the side of making the PR too small, rather than too big. It reduces the chance for you as the
+  author to overlook issues. Small PRs are easier to test and easier to review.
 - DO give PRs short but descriptive names.
 - DO write a useful but brief description of what the PR is for.
 - DO refer to any relevant issues and use keywords that automatically close issues when the PR is merged.
 - DO ensure each commit successfully builds. The entire PR must pass all checks before it will be merged.
-- DO address PR feedback in additional commits instead of amending.
+- DO link the correct Github issue.
+- DO address PR feedback in additional commits instead of amending existing commits.
+- DO NOT add any changes that go beyond what is requested by the reviewers.
 - DO assume that Squash and Merge will be used to merge the commits unless specifically requested otherwise.
-- DO NOT submit "work in progress" PRs. A PR should only be submitted when it is considered ready for review.
-- DO NOT mix independent and unrelated changes in one PR.
 
-To enable good auto-generated changelogs, we prefix all PR titles with a category string, like "BUG: Out of bounds error when using small images".
+To enable good auto-generated changelogs, we prefix all PR titles with a category string, like
+"BUG: Out of bounds error when using small images".
 Those category prefixes must be in upper case, followed by a colon (`:`). Valid categories are
 
 - `ENH` for enhancements, new capabilities
@@ -36,11 +43,17 @@ your dev box:
 - DO write unit tests for each new function or class that you add.
 - DO extend unit tests for existing functions or classes if you change their core behaviour.
 - DO ensure that your tests are designed in a way that they can pass on the local box, even if they are relying on
-specific cloud features.
+  specific cloud features.
+- DO add tests for each bug you fix. When fixing a bug, the suggested workflow is
+  to first write a unit test that shows the invalid behaviour, and only then start to code up the fix.
 - DO run all unit tests on your dev box before submitting your changes. The test suite is designed to pass completely
-also outside of cloud builds.
+  also outside of cloud builds. If you are not
+  a member of the core InnerEye team, note that you may not be able to run some of the unit tests that access the team's
+  AzureML workspace. A member of the InnerEye team will be happy to assist then.
 - DO NOT rely only on the test builds in the cloud. Cloud builds trigger AzureML runs on GPU
-machines that have a higher CO2 footprint than your dev box.
+  machines that have a higher CO2 footprint than your dev box.
+
+More details, in particular on tests that require GPUs, can be found in the [testing documentation](testing.md).
 
 ## Creating issues
 

docs/source/md/software_design_overview.md

@@ -0,0 +1,110 @@
+# Software Development Process
+
+This document provides a high-level overview of the software development process that our team uses for their
+repositories (most importantly [InnerEye-DeepLearning](https://github.com/microsoft/InnerEye-DeepLearning) and
+[InnerEye-Gateway](https://github.com/microsoft/InnerEye-Gateway)).
+
+For detailed guidance for developers, please refer to the [coding guidelines](coding_guidelines.md).
+
+## Version Control
+
+Software code versioning is done via GitHub. The code with the highest level of quality control is in the "main" branch.
+Ongoing development happens in separate branches. Once development in the branch is finished, a Pull Request (PR)
+process is started to integrate the branch into "main".
+
+## Development Process
+
+The design and development of the software in this repository is roughly separated into an **initiation**,
+**prototyping**, and a **finalization** phase. The initiation phase can be skipped for minor changes, for example an
+update to documentation.
+
+### Initiation
+
+During the initiation phase, the following steps are carried out:
+
+- Collection of a set of requirements.
+- Creating a suggested design for the change.
+- Review of the design with member of the core team.
+
+The deliverables of this phase are a detailed design of the proposed change in a GitHub Issue or a separate document.
+
+### Prototyping
+
+The engineering owner of the proposed change will create a branch of the current codebase. This branch is separate from
+the released (main) branch to not affect any current functionality. In this branch, the engineer will carry out the
+changes as proposed in the design document.
+
+The engineer will also add additional software tests at different levels (unit tests, integration tests) as appropriate
+for the design. These tests ensure that the proposed functionality will be maintained in the future.
+
+The deliverable of this phase is a branch in the version control system that contains all proposed changes and a set of
+software tests.
+
+### Finalization
+
+At this point, the engineering owner of the proposed change has carried out all necessary changes in a branch of the
+codebase. They will now initiate a Pull Request process that consists of the following steps:
+
+- The code will be reviewed by at least 2 engineers. Both need to provide their explicit approval before the proposed
+  change can be integrated in the "main" branch.
+- All unit and integration tests will start.
+- All automatic code checks will start. These checks will verify the following:
+
+  - Consistency with static typing rules.
+  - Ensure that no passwords or other types of credentials are revealed.
+  - Ensure that none of the used third-party libraries contains high severity software vulnerabilities that could affect
+    the proposed functionality.
+
+For code to be accepted into the "main" branch, the following criteria need to be satisfied:
+
+- All unit and integration tests pass.
+- The code has been reviewed by at least 2 engineers who are members of the core development team. This review will
+  also assess non-quantifiable aspects of the proposed change, such as clarity and readability of the code and quality
+  of the documentation.
+- Any comments that have been added throughout the review process need to be resolved.
+- All automated checks pass.
+
+Once all the above criteria are satisfied, the branch will be merged into "main".
+
+## Software Configuration Management
+
+Third party libraries (sometimes called Software of Unknown Provenance, SOUP) are consumed via two
+package management systems:
+
+- Conda
+- PyPi
+
+Both of those package management systems maintain strict versioning: once a version of a package is published, it
+cannot be modified in place. Rather, a new version needs to be released.
+
+Our training and deployment code uses Conda environment files that specify an explicit version of a dependent package to
+use (for example, `lightning_bolts==0.4.0`). The Conda environment files are also version controlled in GitHub. Any
+change to a version of a third party library will need to be carried out via the same change management process as a code
+change, with Pull Request, review, and all tests passing.
+
+The list of third party software is maintained in GitHub in the Conda configuration file, that is `environment.yml` for
+Linux environments and `environment_win.yml` for Windows environments. For example, [this is the latest version of the
+environment file for the InnerEye-DeepLearning
+repository](https://github.com/microsoft/InnerEye-DeepLearning/blob/main/environment.yml).
+
+## Defect Handling
+
+The handling of any bugs or defects discovered is done via GitHub Issues.
+
+When an Issue is created, the team members will get notified. Open Issues are kept in a list sorted by priority. For
+example, [this is the list for all of the InnerEye-related Issues](https://github.com/orgs/microsoft/projects/320).
+Priorities are re-assesed regularly, at least once a week.
+
+The Issue(s) with highest priority are assigned to an engineer. The engineer will then analyze the problem, and
+possibly request more information to reproduce the problem. Requesting more information is also handled in the GitHub
+Issue. Once the problem is clearly reproduced, the engineer will start to write a fix for the Issue as well as a test that
+ensures that the fix does indeed solve the reported problem.
+
+Writing the fix and test will follow the process outlined above in the [Prototyping](#prototyping) and
+[Finalization](#finalization) sections. In particular, the fix and test will undergo the Pull Request review process
+before they are merged into the main branch.
+
+## Updates to the Development Process
+
+The development process described here is subject to change. Changes will undergo the review process described in this
+very document, and will be published on GitHub upon completion.