Contributions Guidelines and Tips¶
Contributions of additional tests and code are always welcome. If in doubt, and/or for advice on approaching a particular problem, please contact the projects members (see section _collaboration) Before submitting code, please review the git repository configuration guidelines.
To submit changes, please follow these instructions. Please allow up to two weeks for a maintainer to pick up and review your changes. Though, if you’d like help at any stage, feel free to post on the mailing lists and reference your pull request.
Please edit the documentation directly to correct any minor inaccuracies or to clarify items. The preferred markup syntax is ReStructuredText, keeping with the conventions and style found in existing documentation. For any graphics or diagrams, web-friendly formats should be used, such as PNG or SVG.
Avoid using ‘you’, ‘we’, ‘they’, as they can be ambiguous in reference documentation. It works fine in conversation and e-mail, but looks weird in reference material. Similarly, avoid using ‘unnecessary’, off-topic, or extra language. For example in American English, “Rinse and repeat” is a funny phrase, but could cause problems when translated into other languages. Basically, try to avoid anything that slows the reader down from finding facts.
Rules for Reviewers¶
- Everyone who has experiences in the project is encouraged to review PRs.
- Respectful, kind, patient to the coders
- Freely deny for changes the codebase does not want/need even though perfect design/codes
- Ask questions rather than make statements.
- Not encourage the “Why” questions. Good practice: e.g. Wouldn’t it make more sense to Would you like to? Could you give the reason that …?
- Remember to praise.
- Remember that there is often more than one way to approach a solution.
- Given clear and useful comments, and explain the reason why reqest change
- In general, reviewers should favor approving a PR once it is in a state where it definitely improves the overall code health of the system being worked on, even if the PR isn’t perfect (maintainability, readability, and understandability).
- Share your best practice/knowledge as a mentor.
- Cautiously regard personal preference as best practice and impose to contributors
- Dismiss your approval if you add new comment for a PR after you already have given an approval
- It is ok to use a ‘request review’ tool to ask someone to review as you like, but not a must.
- It is ok to cancel the request review to you if you think you are not a suitable one for this PR.
- Wait for request review for no more than 2 weeks on those PRs which have already 2 approvals
Rules for Maintainers¶
- Includes all reviewer’s rules
- Make sure all PRs submitted can be closed within 3 months
- Generally every PR needs at least 1 maintainer’s approval and total 2 approvals before being merged.
- Add request for review for more maintainers if there is a need
- Add a comment to explain why a PR need the label ‘request_2_maintainers’
- Mark proper and necessary labels according to the information provided by the PR contributor
- Closing a PR threshold:no response from contributor after 1 month, the PR will be labelled as, “No response” and after 3 months it will be closed
Rules for Contributors¶
- [Must] Coding style compliance, for example, align with inspekt tool, pep8
- [Must] PR commit message is meaningful. Refer to the link on how to write a good commit message
- [Must] Travis CI pass and no conflict
- [Must] Provide test results. If no, provide justification. Apply to any PR
One of below options should be aligned:
- [Must] If the function defined with right docstring (description and params, and return if have)
- [Must] If the PR depends on other PRs, please add a comment to say if your PR has a dependence in order to ensure the PR is merged after dependence PRs
- [Must] If the API of one library is changed, ask for all test cases to be modified which invoke this library and provide test results of representative test cases.
- [Must] If the case does some package version judgement for the new case support or compatible backwards
- [Must] If the test code have suitable env backup and recovery steps
- [Optional] If have the necessary and clear comments for the code explanation for steps
- [Optional] If the case is applied to multiple arches.
- [Optional] If have duplication, need to create new function or reuse existing library in avocado/avocado-vt
- [Optional] If the logic are complete and no important branches which are not dealt with
- [Optional] If the code seems clear and concise, define functions to increase the readability
- [Optional] Use python supported library instead of shell cmd running by process.run if possible
- [Optional] If the feature test related aspects are correct
- [Optional] Add comments to ask questions which you do not understand
- [Optional] Pay more attention to ‘test.fail(xxx)’ or exception raise part, such as if there is log info
- [Optional] Reply to the comment when you have fixed the comment (see good sample in Appendix.4)
- [Optional] Better to use @Someone to ask for review when your PR is submitted
- [Optional] Use ‘request review’ to ask the original reviewer to request again when you finish updates