forked from avocado-framework/avocado-vt
-
Notifications
You must be signed in to change notification settings - Fork 0
/
Copy pathPR_Review_And_Contribute_Practices.txt
60 lines (53 loc) · 4.14 KB
/
PR_Review_And_Contribute_Practices.txt
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
For reviewers rules:
============================================================================
1)Everyone who has experiences in the project is encouraged to review PRs.
2)Respectful, kind, patient to the coders
3)Freely deny for changes the codebase does not want/need even though perfect design/codes
4)Ask questions rather than make statements.
5)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 ...?
6)Remember to praise.
7)Remember that there is often more than one way to approach a solution.
8)Given clear and useful comments, and explain the reason why reqest change
9)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)
10)Share your best practice/knowledge as a mentor.
11)Cautiously regard personal preference as best practice and impose to contributors
12)Dismiss your approval if you add new comment for a PR after you already have given an approval
13)It is ok to use a 'request review' tool to ask someone to review as you like, but not a must.
14)It is ok to cancel the request review to you if you think you are not a suitable one for this PR.
15)Wait for request review for no more than 2 weeks on those PRs which have already 2 approvals
Maintainer rules:
=============================================================================
1)includes all reviewer's rules
2)Make sure all PRs submitted can be closed within 3 months
3)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'
4)Mark proper and necessary labels according to the information provided by the PR contributor
5)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
Contributors Rules:
============================================================================
1)[Must] Coding style compliance, for example, align with inspekt tool, pep8
2)[Must] PR commit message is meaningful. Refer to the link on how to write a good commit message
3)[Must] Travis CI pass and no conflict
4)[Must] Provide test results. If no, provide justification. Apply to any PR
One of below options should be aligned:
5)[Must] If the function defined with right docstring (description and params, and return if have)
6)[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
7)[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.
8)[Must] If the case does some package version judgement for the new case support or compatible backwards
9)[Must] If the test code have suitable env backup and recovery steps
10)[Optional] If have the necessary and clear comments for the code explanation for steps
11)[Optional] If the case is applied to multiple arches.
12)[Optional] If have duplication, need to create new function or reuse existing library in avocado/avocado-vt
13)[Optional] If the logic are complete and no important branches which are not dealt with
14)[Optional] If the code seems clear and concise, define functions to increase the readability
15)[Optional] Use python supported library instead of shell cmd running by process.run if possible
16)[Optional] If the feature test related aspects are correct
17)[Optional] Add comments to ask questions which you do not understand
18)[Optional] Pay more attention to ‘test.fail(xxx)’ or exception raise part, such as if there is log info
19)[Optional] Reply to the comment when you have fixed the comment (see good sample in Appendix.4)
20)[Optional] Better to use @Someone to ask for review when your PR is submitted
21)[Optional] Use ‘request review’ to ask the original reviewer to request again when you finish updates