Skip to content

Commit db9a536

Browse files
reactive-firewallreactive-firewall
committed
[DOCUMENTATION] Minor update to CONTRIBUTING guide.
* These changes cause GHI #412 Changes in file .github/CONTRIBUTING.md: * reworded the review checklist * related work
1 parent f03cc4c commit db9a536

File tree

1 file changed

+199
-56
lines changed

1 file changed

+199
-56
lines changed

‎.github/CONTRIBUTING.md‎

Lines changed: 199 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
1-
## Contributing
1+
# Contributing
2+
23
You are here to help the Python Multicast Project? Awesome, feel welcome and read the following sections in order to know what and how to work on something. If you get stuck at any point you can create a ticket on GitHub.
34

45
Following these guidelines helps to communicate that you respect the time of the developers managing and developing this open source project. In return, they should reciprocate that respect in addressing your issue, assessing changes, and helping you finalize your pull requests.
@@ -11,129 +12,269 @@ For all contributions, please respect the following guidelines:
1112

1213
Each pull request should implement ONE feature or bugfix. If you want to add or fix more than one thing, submit more than one pull request.
1314
- Do not commit changes to files that are irrelevant to your feature or bugfix (eg: .gitignore).
15+
<details><summary>Always use git stash instead</summary>
16+
17+
```bash
18+
# remember your starting branch
19+
VCS_BRANCH_NAME=$(${GIT:-git} name-rev --name-only HEAD | cut -d~ -f1-1) ;
20+
git add -p $(git ls-files -m) # add modified files interactivly
21+
# select ONLY the changes you are working on
22+
git stash push # stash the intended changes
23+
git add $(git ls-files -m) # add remaining modified files automaticly
24+
git branch my-idea-untracked # create a new branch where you are
25+
git switch my-idea-untracked # switch to the new branch
26+
git commit # commit the unrelated changes on the new branch
27+
git switch ${VCS_BRANCH_NAME} # change back to the starting branch
28+
git stash pop # unstash the intended changes
29+
```
30+
31+
</details>
32+
1433
- Do not add new external dependencies unless ABSOLUTELY necessary (these could cause Load Errors on certain systems).
1534
- Add new tests for new code if able (otherwise add descriptive pseudo-tests as issues).
1635
- Be willing to accept criticism and work on improving your code; care must be taken not to introduce bugs or vulnerabilities.
1736

18-
#### Known Issues and Possible Improvements
37+
## Known Issues and Possible Improvements
1938

2039
For Details on current issues:
2140
- see [Issues](https://github.com/reactive-firewall/multicast/issues)
2241

23-
## Triaging tickets
42+
### Triaging tickets
2443
Here is a brief explanation on how I triage incoming tickets to get a better sense of what needs to be done on what end.
2544

26-
>>Note
27-
>>
28-
>>You will need Triage permission on the project in order to do this. You can ask one of the members of the Team to give you access. (Please join the beta first.)
45+
> [!NOTE]
46+
> You will need Triage permission on the project in order to do this. You can ask one of the
47+
> core maintainers to give you access.
48+
49+
### Initial triage
50+
When sitting down to do some triaging work, start with the list of untriaged tickets. Consider
51+
all tickets that do not have a label as untriaged. The first step is to categorize the ticket into
52+
one of the following categories and either close the ticket or assign an appropriate label.
53+
54+
#### If the reported issue . . .
55+
56+
##### is not valid
57+
58+
If you think the ticket is invalid comment why you think it is invalid, then close the ticket.
59+
Tickets might be invalid if they were already fixed in the past or it was decided that the proposed
60+
feature will not be implemented because it does not conform with the overall goal of Multicast
61+
Project. Also if you happen to know that the problem was already reported, label the ticket with
62+
Status: duplicate, reference the other ticket that is already addressing the problem and close
63+
the duplicate.
64+
65+
**Examples**:
66+
67+
[This is an issue. (A good starting point for first person to discover)](https://github.com/reactive-firewall/multicast/issues/412)
68+
69+
70+
##### does not provide enough information
2971

30-
## Initial triage
31-
When sitting down to do some triaging work, start with the list of untriaged tickets. Consider all tickets that do not have a label as untriaged. The first step is to categorize the ticket into one of the following categories and either close the ticket or assign an appropriate lable. The reported issue
72+
Add the label question if the reported issue does not contain enough information to decide if
73+
it is valid or not and ask (eg. by commenting on the ticket) for the required information to move
74+
forward. We will re-triage all tickets that have the label question assigned. If the original
75+
reporter left new information we can try to re-categorize the ticket. If the reporter did not come
76+
back to provide more required information after a long enough time, we will close the ticket
77+
(this should be roughly about two weeks, but can be left open for longer).
3278

33-
* is not valid
34-
If you think the ticket is invalid comment why you think it is invalid, then close the ticket. Tickets might be invalid if they were already fixed in the past or it was decided that the proposed feature will not be implemented because it does not conform with the overall goal of Multicast Project. Also if you happen to know that the problem was already reported, label the ticket with Status: duplicate, reference the other ticket that is already addressing the problem and close the duplicate.
79+
**Examples**:
3580

36-
Examples:
81+
> My builds stopped working. Please help!
3782
38-
This is an issue. (A good starting point for first person to discover)
83+
* Ask for a link to the build log and for which version is affected.
3984

4085

41-
* does not provide enough information
42-
Add the label question if the reported issue does not contain enough information to decide if it is valid or not and ask on the ticket for the required information to go forward. I will re-triage all tickets that have the label question assigned. If the original reporter left new information we can try to re-categorize the ticket. If the reporter did not come back to provide more required information after a long enough time, we will close the ticket (this should be roughly about two weeks).
86+
##### is a valid enhancement proposal
4387

44-
Examples:
88+
If the ticket contains an enhancement proposal (eg. a new feature, an idea to improve an exsisting
89+
feature, etc.) that aligns with the goals of Multicast, then add the label `Enhancement`. If the
90+
proposal seems valid but requires further discussion between core contributors because there
91+
might be different possibilities on how to implement the enhancement, then also add the
92+
label question.
4593

46-
My builds stopped working. Please help! Ask for a link to the build log and for which project is affected.
94+
**Examples**:
4795

96+
>Improve documentation Examples in contribute docs.
4897
49-
* is a valid enhancement proposal
50-
If the ticket contains an enhancement proposal that aligns with the goals of Multicast, then add the label Enhancement. If the proposal seems valid but requires further discussion between core contributors because there might be different possibilities on how to implement the enhancement, then also add the label question.
98+
> Provide better integration with security feature XYZ
5199
52-
Examples:
100+
> Refactor module X for better readability (see #387)
53101
54-
Improve documentation Examples in contribute docs.
55-
Provide better integration with security feature XYZ
56-
Refactor module X for better readability (see #48)
57-
Achieve world domination (also needs the label question)
102+
> Achieve world domination
103+
_(also needs the label question)_
58104

105+
##### is a valid problem within the code base
59106

60-
* is a valid problem within the code base:
61107
If it’s a valid bug, then add the label Bug. Try to reference related issues if you come across any.
62108

63-
Examples:
109+
**Examples**:
64110

65111
Multicast Library fails to transmit the leter 'x' or 'y' (logs attached)
66112

67113

68-
* is a question and needs answering:
69-
If the ticket contains a question about Multicast messages or the code, then move question to FAQ on wiki and add task to answer question on wiki to the issue (or close if already answered).
114+
##### is a question and needs answering
70115

71-
Examples:
116+
If the ticket contains a question about Multicast messages or the code, then move the question to
117+
the FAQ documentation and add a task to answer the question to the issue (or close if already
118+
answered).
72119

73-
My message was seen by two hosts. Why?
74-
Why are my builds failing?
120+
**Examples**:
75121

122+
> My message was seen by two hosts. Why?
76123
77-
Helpful links for triaging
78-
Here is a list of links for contributors that look for work:
124+
> Why are my builds failing?
79125
80-
Untriaged tickets: Go and triage them!
81126

127+
### Helpful links for triaging
82128

83-
# Reviewing:
129+
Here is a list of links for contributors that are looking to help triage:
84130

85-
This is the checklist that I try to go through for every single pull request that I get. If you're wondering why it takes so long for me to accept pull requests, this is why.
131+
* [Untriaged issues](https://github.com/reactive-firewall/multicast/issues?q=is%3Aissue%20sort%3Acomments-asc%20state%3Aopen%20no%3Alabel): Go and triage them!
132+
* [Good first issues](https://github.com/reactive-firewall/multicast/issues?q=is%3Aissue%20state%3Aopen%20label%3A%22good%20first%20issue%22%20sort%3Acomments-asc)
133+
* [All Open Issues](https://github.com/reactive-firewall/multicast/issues)
86134

135+
---
136+
# Reviewing Checklist:
137+
138+
> [!NOTE]
139+
> This is the checklist that I try to go through for every single release.
140+
> If you're wondering why it takes so long for me to complete stable releases, this is why.
141+
142+
---
87143
## General
88144

89-
- [ ] Is this change useful to me, or something that I think will benefit others greatly?
90-
- [ ] Check for overlap with other PRs.
91-
- [ ] Think carefully about the long-term implications of the change. How will it affect existing projects that are dependent on this? How will it affect my projects? If this is complicated, do I really want to maintain it forever? Is there any way it could be implemented as a separate package, for better modularity and flexibility?
145+
> [!TIP]
146+
> Ask yourself "Is this change useful to me, or something that I think will benefit others greatly?" before opening the PR.
92147
148+
- [ ] Check for overlap with other PRs.
149+
> Those PRs are . . . _include a list_.
150+
* for example
151+
> ## Included and Superseded PR/MRs
152+
>
153+
> * Supersedes . . . _link to pull-request_
154+
> * Includes and Supersedes _link to pull-request_
155+
156+
**OR**
157+
158+
> This will introduce . . . _description of what is changed only by this PR's set of changes_.
159+
160+
- [ ] Check for relevant GHI.
161+
* For example:
162+
> - [x] Closes _link to issue_
163+
> - [ ] Contributes to _link to issue_
164+
165+
- [ ] Think carefully about the long-term implications of the change.
166+
- [ ] How will it affect existing projects that are dependent on this?
167+
* For example:
168+
> Breaking changes include . . . _include a list of impacts to dependents_
169+
170+
**OR**
171+
> This _PR type, ie version|update|patch_ . . . _previous version this can safely upgrade from_.
172+
- [ ] How will it affect my projects? If this is complicated, do I really want to maintain it forever?
173+
* For example:
174+
> - [ ] Opens _link to new issue_
175+
- [ ] Is there any way it could be implemented as a separate package, for better modularity and flexibility?
176+
- [ ] yes
177+
> . . . _plan for how goes here_
178+
179+
* include the following advisory at the bottom of your plan.
180+
> 🚧 CAUTION: 🏗️ This is still experimental. ⛔ DO NOT MERGE AS IS. :no_good:
181+
182+
**OR**
183+
- [ ] No
184+
185+
---
93186
## Check the Code
94187

95188
- [ ] If it does too much, ask for it to be broken up into smaller PRs.
189+
* For example:
190+
> This is too monolithic; please consider breaking this PR into parts.
191+
>
192+
> These numerous changes are disjoint; please reorganize into separate, smaller PRs.
193+
96194
- [ ] Does it pass `make test-style` (flake8, etc.)?
195+
> These changes follow the current project style conventions as tested by `make test-style`.
196+
197+
**See project `Makefile`.**
198+
199+
**OR**
200+
201+
- [ ] Comment on the PR describing style convention violations.
202+
* Include the following advisory until it is fixed:
203+
> 🚧 CAUTION: 🏗️ This is still experimental. 🔧 Fix in progress. ⛔ DO NOT MERGE AS IS.
204+
97205
- [ ] Is it consistent?
206+
> This change follows current project style conventions as tested by `make test-style`.
207+
208+
**See project `Makefile`.**
209+
210+
**OR**
211+
212+
- [ ] Comment on the PR describing style convention violations.
213+
* Include the following advisory until it is fixed:
214+
> 🚧 CAUTION: 🏗️ This is still experimental. 🔧 Fix in progress. ⛔ DO NOT MERGE AS IS.
215+
98216
- [ ] Review the changes carefully, line by line. Make sure you understand every single part of every line. Learn whatever you do not know yet.
99-
- [ ] Take the time to get things right. PRs almost always require additional improvements to meet the bar for quality. Be very strict about quality. This usually takes several commits on top of the original PR.
100217

218+
🗒️ **Take the time to get things right. PRs almost always require additional improvements to meet the bar for quality. Be very strict about quality. This usually takes several commits on top of the original PR.**
219+
220+
---
101221
## Check the Tests
102222

103-
- [ ] Does it have tests? If not:
223+
- [ ] Does it have tests for all the changes? If not:
224+
* Comment on the PR:
225+
> Can you please add tests for this code to `tests/test_blah.py`?
104226
105-
- [ ] Comment on the PR "Can you please add tests for this code to `tests/test_blah.py`", or...
106-
- [ ] Write the tests yourself.
227+
**OR**
107228

108-
- [ ] Do the tests pass for all of the following? If not, write a note in the PR, or fix them yourself.
229+
- [ ] Write the tests yourself.
109230

110-
- [ ] Python 3.12 - Mac (OPTIONAL)
111-
- [ ] Python 3.11 - Linux
112-
- [ ] Python 3.12 - Linux
113-
- [ ] Python 3.9 (or Newer) - Mac
114-
- [ ] Python 3.10 - Linux
115-
- [ ] Python any - Windows (OPTIONAL)
231+
- [ ] Do the tests pass for all of the CI tests? If not:
232+
* Comment on the PR requesting a fix for the regression in CI.
116233

234+
**OR**
235+
236+
- [ ] Fix them yourself.
237+
238+
**AND**
239+
240+
- Include the following advisory until it is fixed:
241+
> 🚧 CAUTION: 🏗️ This is still experimental. 🔧 Fix in progress. ⛔ DO NOT MERGE AS IS.
242+
243+
---
117244
## Check the Docs
118245

119246
- [ ] Does it have docs? If not:
247+
* Comment on the PR:
248+
> Can you please add docs for this feature to the documentation?
249+
250+
**OR**
120251

121-
- [ ] Comment on the PR "Can you please add docs for this feature to the wiki", or...
122-
- [ ] Write the docs yourself.
252+
- [ ] Write the docs yourself.
123253

124254
- [ ] If any new functions/classes are added, do they contain docstrings?
125255

256+
---
126257
## Credit the Authors
127258

128-
- [ ] Add name and URL to `README.md` for security fixes.
259+
- [ ] Add name and URL to documentation for security fixes.
129260
- [ ] Thank them for their hard work.
261+
- [ ] Close Issues.
130262

131-
## Check the copyright year:
263+
---
264+
# Release Checklist:
132265

133-
- [ ] reads:
266+
## Release (optional)
134267

135-
> Copyright (c) 2017-2025, Author
268+
> **Decide whether the changes in master make sense as a major, minor, or patch release. Look at the clock. If you're tired, release later when you have time to deal with release problems.**
136269
270+
- [ ] Merge the PR branch. This will close the PR's issue.
271+
- [ ] Close any duplicate or related issues that can now be closed. Write thoughtful comments explaining how the issues were resolved.
272+
273+
## Check the copyright year:
274+
275+
- [ ] Verify it reads:
276+
277+
> Copyright (c) 2017-2025, _Author_
137278
138279
## Close Issues
139280

@@ -144,4 +285,6 @@ This is the checklist that I try to go through for every single pull request tha
144285

145286
- [ ] Decide whether the changes in master make sense as a major, minor, or patch release.
146287
- [ ] Look at the clock. If you're tired, release later when you have time to deal with release problems.
147-
- [ ] Then follow all the steps in [EXAMPLE Release Checklist](https://github.com/reactive-firewall/Pocket-PiAP/issues/87) FIXME!
288+
- [ ] Then follow all the steps in [EXAMPLE Release Checklist](link to be created).
289+
290+
---

0 commit comments

Comments
 (0)