Contributor Guidance
This is feedback from contributors who review code contributions and contains some suggestions that could make the role of reviewer easier. These are suggestions for discussion and not rules (yet).
Suggestion here: Follow the guidelines below when opening a Draft Pull Request. Once you have run the checks in the draft pull request, select Ready for Review.
- Ensure a descriptive title. e.g. Issue # + Description
- Tag Issues and PRs in linked Pull Request section
- Tag the milestone as specified in the originating Issue
- Add labels, where appropriate to aid in prioritization
- Make sure that All Checks (DCO and Jenkins Run) are okay. If there are issues that you cannot handle, comment it in the PR.
- Please try and address only one fix per PR. Multiple fixes in one PR makes things more complicated for everyone.
- PR must relate to an issue agreed during Planning for the target milestone or in the prioritized backlog. I concede this may not apply to external contributors as we are always grateful for new contributors and don't want to put anyone off getting involved.
- Please... No direct messaging to solicit review/merging.
- Reduce number of commits per PR. It can be frustrating if requested to review and subsequently complete the review request only to find several additional commits have taken place.
- If raising the PR to see if checks pass or as WIP raise as a Draft. It will not be reviewed and when you are comfortable with it you can close the draft and raise again for review
- Aim for a minimum of 2 approvals rather than 1.
- Prioritize merging of PR's based on complexity and target milestone.
- Generally merging will be by agreement of several reviewers rather than one individual.
- When filtering the PR list use recommend using is:pr is:open status:success rather than waste time reviewing PR's that are incomplete.
The addition of an interface allows the code base to move to a more maintainable OO style based around, classes and methods rather than functions. To this end no code should be added as functions in extension.ts or the ..Actions.ts files. Code migration is still underway to create generic command related methods and some issues experienced merging changes previously however we should now be able to manage at a function to method level now the previous changes are solidified.
- Effort is underway to include System test into pipeline build.
- Unit test code coverage. Must maintain +90% currently dropped to 91% using CodeCov may be problematic so before commiting you should check your local unit test coverage matrix. For example the % Lines for the All Files row
- We should not rest until all files have >90% lines covered