You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
This repository was archived by the owner on Sep 7, 2020. It is now read-only.
documentation: Adapt CONTRIBUTING.md for the move to gitlab
This is an attempt at keeping the docs up to date with the move from github to gitlab.
Since this document will be used as a template for prplWrt as well, it's double important that it stays up to date.
I anticipate we'll need another update once the move is complete and we've settled on how to work effectively with gitlab a bit more.
The "Merge request workflow" in particular might change significantly, but I don't want to do it yet because I'm not certain yet what it will look like.
Also, I don't know how (if) the DCO check still works with gitlab, or if we need to tweak things a little there.
No doubt we'll discover other issues.
So, this is just an *initial version* of the contribution guidelines, adapted for gitlab.
See PPM-335
Signed-off-by: Frederik Van Bogaert <frederik.vanbogaert@mind.be>
Copy file name to clipboardExpand all lines: CONTRIBUTING.md
+43-43Lines changed: 43 additions & 43 deletions
Display the source diff
Display the rich diff
Original file line number
Diff line number
Diff line change
@@ -4,13 +4,14 @@ You can contribute to prplMesh in various ways: reporting bugs, improving docume
4
4
5
5
## Getting involved
6
6
7
-
A large part of the conversation takes place directly on github, through issues, pull requests, and comments on commits.
8
-
Thus, it is advisable to subscribe to (i.e. watch) the issues you are interested in, or the project as a whole.
7
+
Part of the conversation takes place directly on gitlab through merge requests, and comments on commits.
8
+
But discussions about the design of features or the approach to bug fixes happens in JIRA.
9
+
Thus, it is advisable to subscribe to (i.e. watch) the JIRA issues you are interested in.
9
10
10
11
Daily conversations take place on [Slack](https://prplfoundation.slack.com/).
11
-
You can ask for an invite to anyone currently involved, e.g. by sending a message to an active contributer on github.
12
+
You can ask for an invite to anyone currently involved, e.g. by sending a message to an active contributer on gitlab.
12
13
13
-
Some issues are labelled as [good first issue](https://github.com/prplfoundation/prplMesh/issues?q=is%3Aopen+is%3Aissue+label%3A%22good+first+issue%22).
14
+
Some issues are labelled as [good first issue](https://jira.prplfoundation.org/browse/PPM-305?jql=labels %3D good_first_issue).
14
15
Picking one of these to work on as your first contribution is recommended.
15
16
16
17
The [prpl Foundation Code of Conduct](https://prplfoundation.org/about/code-of-conduct/) applies to all communication about prplMesh.
@@ -31,10 +32,10 @@ It currently resides as pptx and docx documents in the [documentation](documenta
31
32
## Contributing code
32
33
33
34
Before development is started, a [JIRA task or bug](#working-with-jira) should have been created for the issue.
34
-
All development is managed using [pull requests](#pull-requests-workflow).
35
+
All development is managed using [merge requests](#merge-requests-workflow).
35
36
[Commits](#commits) must be in logical, consistent units and have a good commit message that includes the JIRA key of the corresponding issue.
36
37
Every commit must carry a Signed-off-by tag to assert your agreement with the [DCO](#developers-certificate-of-origin).
37
-
Pull requests are reviewed and checks are performed on them before they are merged.
38
+
Merge requests are reviewed and checks are performed on them before they are merged.
38
39
Code should adhere to the [coding style](#coding-style).
39
40
40
41
Note that everybody follows these same guidelines.
@@ -57,10 +58,10 @@ Time spent reviewing other issues is not counted, but time spent implementing re
57
58
When it's time for review, please transition the issue manually to the "In review" state.
58
59
We're looking at possibly automating this in the future, but it still needs to be done manually for now.
59
60
Make sure the JIRA issue has an informative description; it will form the basis of any discussion of the feature or bug in question, so you want to make sure that the description makes the problem clear.
60
-
You can then copy the description to the pull request description (it's OK if they're the same).
61
+
You can then copy the description to the merge request description (it's OK if they're the same).
61
62
You may need to adjust the syntax manually if you do this, though.
62
63
63
-
JIRA uses a syntax that's different from the markdown used by github/gitlab. It's described [here](https://jira.prplfoundation.org/secure/WikiRendererHelpAction.jspa)
64
+
JIRA uses a syntax that's different from the markdown used by gitlab. It's described [here](https://jira.prplfoundation.org/secure/WikiRendererHelpAction.jspa)
64
65
65
66
### Copyright
66
67
@@ -108,7 +109,7 @@ A commit message consists of a subject, a message body and a set of tags.
108
109
Co-Authored-by: The Other Author <email@address.com>
109
110
Signed-off-by: The Other Author <email@address.com>
110
111
111
-
The subject must be very short, it appears in the [short log](https://github.com/prplfoundation/prplMesh/commits/master).
112
+
The subject must be very short, it appears in the [short log](https://gitlab.com/prpl-foundation/prplmesh/prplMesh/-/network/master).
112
113
113
114
* Write it in the imperative: "add support for X".
114
115
* Start with a prefix that indicates the component: tlvf, common, documentation, bpl, bwl, bcl, bml, transport, topology, master, slave, monitor.
@@ -187,16 +188,18 @@ This is a short way for you to say that you are entitled to contribute the patch
187
188
It is a legal statement that asserts your agreement with the [DCO](#developers-certificate-of-origin).
188
189
Therefore, it must have your *real name* (First Last) and a valid e-mail address.
189
190
Adding this tag can be done automatically by using `git commit -s`.
190
-
If you are editing files and committing through GitHub, you must write your real name in the “Name” field in GitHub settings and the email used in the "Signed-off-by:" must be your primary github email.
191
-
You must manually add the "Signed-off-by:" to the commit message that github requests.
191
+
If you are editing files and committing through GitLab, you must write your real name in the “Full Name” field in your GitLab profile and the email used in the "Signed-off-by:" must be your "Commit email" address.
192
+
You must manually add the "Signed-off-by:" to the commit message that gitlab requests.
192
193
If you are editing files and committing on your local PC, set your name and email with:
193
194
194
195
```bash
195
196
git config --global user.name "my name"
196
197
git config --global user.email "my@email.address"
197
198
```
198
199
199
-
### Pull Request workflow
200
+
Then, adding the "Signed-off-by" line is as simple as using `git commit -s` instead of `git commit` (using an alias is recommended, e.g. `git config --global alias.ci='commit -s'`)
201
+
202
+
### Merge Request workflow
200
203
201
204
This section describes the workflow followed by core contributors and maintainers.
202
205
It is advised to follow a similar workflow for your own contributions.
@@ -206,44 +209,43 @@ For smaller contributions, you may take shortcuts.
206
209
The workflow is explained in detail below. In summary, it consists of these steps.
207
210
208
211
1. Create a branch
209
-
2. Create a draft pull request
212
+
2. Create a "WIP" (work in progress) merge request. This means that the MR title starts with "WIP:", and is treated specially by gitlab (for instance, by preventing it from getting merged while in "WIP" state)
210
213
3. Make the changes, commit with amend and rebase
211
214
4. Push regularly
212
-
5. Clean up the commits, push, and set pull request to "Ready for review"
215
+
5. Clean up the commits, push, and move the MR out of "WIP" state
213
216
6. Review starts - reviewer "Requests changes"
214
217
7. Author addresses review comments in additional fixup commits.
215
218
* If no more fixes are needed
216
-
* Author rebases with `git rebase -i --autosquash master` to clean up the pull request.
219
+
* Author rebases with `git rebase -i --autosquash master` to clean up the merge request.
217
220
* If more fixes are needed (suggested by reviewers or by the author himself)
218
221
* Author does rebase-force-push to squash fixup commits and asks for a followup review (this makes the next review iterations simpler)
219
222
220
223
8. Review is approved by maintainers.
221
-
9. Use `git rebase -i --autosquash master` to clean up the pull request.
224
+
9. Use `git rebase -i --autosquash master` to clean up the branch of the merge request.
222
225
10. If everything is ready, add the "ready for merge" label.
223
-
11. The pull request will be merged automatically when CI succeeds.
226
+
11. The merge request will be merged automatically when CI succeeds.
224
227
225
228
Start by creating a branch.
226
229
We give branches a name following `<type>/<subject>`.
227
230
Types are `feature` for feature development, `bugfix` for fixing bugs from the issues list, `hotfix` for small fixes without an issue, and `dev/<user>` for personal development branches that are not meant for merging.
228
231
Both `bugfix` and `feature` branches should have a JIRA identifier in their branch name (e.g. feature/PPM-204-implement-dynamic-steering)
229
232
230
-
This branch is immediately pushed, and a draft pull request is created for it.
231
-
The pull request can be created with the [`hub`](https://github.com/github/hub) tool: `hub pull-request -d`.
232
-
This draft pull request signals the others that someone is working on this feature/bugfix.
233
+
This branch is immediately pushed, and a "WIP" (Work in progress) merge request is created for it.
234
+
This can be done in a number of ways: when using `git push` on the CLI, you will get a link to create a MR; alternatively, you can create a merge request from the gitlab UI, or [add options to git push](https://docs.gitlab.com/ee/user/project/push_options.html#push-options-for-merge-requests) to instruct gitlab to automatically create a MR for it.
235
+
This WIP merge request signals the others that someone is working on this feature/bugfix.
233
236
It allows others to see what you're doing before it is completed, and to give early feedback and suggestions.
234
-
For such a draft pull request, it is not yet necessary that the commits are nicely split up.
237
+
For such a WIP, it is not yet necessary that the commits are nicely split up.
235
238
236
239
Continue developing the code.
237
240
Push your work regularly.
238
241
You can make separate commits, or amend a single commit, or rebase and sort it into different commits, at your option.
239
242
Every time you push, CI will run on the code and you will be informed of any issues with it.
240
-
Also, even if the pull request is still draft, maintainers may start giving comments on it.
243
+
Also, even if the merge request is still WIP, maintainers may start giving comments on it.
241
244
The purpose of these comments is to make sure your work is aligned with expectations.
242
245
It avoids that after a lot of work, you are asked to still make major changes.
243
246
244
247
Once your feature is ready, use `git rebase -i` to organise it in clean commits and add a proper commit message to every commit, including a Signed-off-by.
245
-
Force-push the branch and change the pull request state from "Draft" to "Ready for review".
246
-
248
+
Force-push the branch and take the merge request out of "WIP" state.
247
249
Other contributors will start reviewing your change and make suggestions for improvements.
248
250
The review has the following goals:
249
251
@@ -255,12 +257,12 @@ The review has the following goals:
255
257
* Identify potential bugs.
256
258
* Identify missing tests.
257
259
258
-
Very often, the review will lead to comments that don't really need to be addressed for the pull request to be accepted.
260
+
Very often, the review will lead to comments that don't really need to be addressed for the merge request to be accepted.
259
261
Many of the review goals are more about having a discussion than about really forcing changes.
260
-
Unfortunately, github doesn't have an easy way to make such a distinction, so reviewers have to mention explicitly when a suggestion is optional.
261
-
If there are review comments that need to be addressed, the pull request will be marked as "Changes requested".
262
+
Unfortunately, gitlab doesn't have an easy way to make such a distinction, so reviewers have to mention explicitly when a suggestion is optional.
263
+
For issues that can be addressed later, it is acceptable to create a new JIRA task for them, then resolve the comment by pointing to the new JIRA task that will address those comments.
262
264
263
-
If you make more changes after a pull request is marked as "Ready for review", do not rebase or amend.
265
+
If you make more changes after a merge request is moved out of "WIP" state, do not rebase or amend.
264
266
This will allow reviewers to easily see the differences compared to their previous review.
265
267
Instead, create additional commits with `git commit --fixup <sha1 of commit to fix> -e`.
266
268
Do _not_ add a Signed-off-by to these commits.
@@ -270,44 +272,43 @@ If the commit message itself needs to be changed, use `git commit --squash <sha1
270
272
Below the subject line, add a second subject line, followed by the complete commit message and the Signed-off-by.
271
273
During the autosquash rebase, we'll only retain the second commit message (see below).
272
274
273
-
In the pull request's web interface, mark the comments that have been addressed as Resolved.
275
+
In the merge request's web interface, mark the comments that have been addressed as Resolved (this should be automatic for specific code related comments; gitlab automatically detects if the code in question has been changed).
274
276
Also mark the comments that are optional as Resolved.
275
277
If you don't take the suggestion directly or you disagree or only apply it in part, don't mark it as Resolved.
276
278
Regularly push your branch.
277
279
278
280
There may be several iterations in the review.
279
-
Finally, the pull-request is marked as Approved by the maintainers.
281
+
Finally, the merge request is marked as Approved by the approvers.
280
282
However, it cannot be applied as is because the commits are still "dirty".
281
283
The last step is to perform `git rebase -i --autosquash master`.
282
284
This will apply the fixup commits automatically.
283
285
For the squash commits, it will stop in an editor to allow you to update the commit message.
284
286
Just use the last commit message, remove all the rest.
285
287
Finally, force-push the branch.
286
288
The DCO check will now succeed.
287
-
Add the "ready for merge" label to the pull request.
288
-
When CI finishes and is successful, the pull request will be merged automatically.
289
+
When CI finishes and is successful, the merge request will be merged automatically.
289
290
290
291
Note that there are a few cases where it is not possible to use fixup commits.
291
292
In these cases, use rebase and create a clean series of commits again.
292
-
Add a comment in the pull request why this was done.
293
+
Add a comment in the merge request why this was done.
293
294
This is needed at least in the following cases:
294
295
295
-
* If you need to pull in changes from master or another branch or pull request.
296
+
* If you need to pull in changes from master or another branch or merge request.
296
297
* If you need to reorder the commits.
297
298
* If you need to move a subset of the changes from one commit to another commit.
298
299
299
300
If you are working on a big feature, you often encounter something small that needs to be fixed or improved that is relatively independent of the feature.
300
-
Such a change can be included as one of the first commits in the pull request.
301
-
However, often it's useful to create a separate pull request for it, so it can be applied more quickly.
301
+
Such a change can be included as one of the first commits in the merge request.
302
+
However, often it's useful to create a separate merge request for it, so it can be applied more quickly.
302
303
The typical workflow for this is:
303
304
304
305
* Make the fix and commit it with a proper commit message.
305
306
* Check out a new branch `hotfix/<description>` based on master.
306
307
* Cherry-pick the fix commit.
307
-
* Push and create a pull request.
308
+
* Push and create a merge request.
308
309
* Check out the development branch and rebase on the hotfix branch. This will automatically remove the fix commit.
309
310
310
-
Sometimes a pull request looks like it's ready for review or merge (it is not draft, it has an approval) but really it isn't.
311
+
Sometimes a merge request looks like it's ready for review or merge (it is not in "WIP", it has an approval) but really it isn't.
311
312
For this purpose, the "don't merge" tag can be added to it.
312
313
Example use cases:
313
314
@@ -317,21 +318,20 @@ Example use cases:
317
318
- It's an oldish PR that needs to be rebased and conflicts need to be resolved.
318
319
- It needs more testing before really approving it for merge.
319
320
320
-
If a PR is marked "don't merge" and it becomes ready (i.e. you did the necessary fixups and rebased on master), please remember to remove the tag again.
321
-
322
-
A good example of a pull request with review, discussion, and several iterations is https://github.com/prplfoundation/prplMesh/pull/831.
321
+
If a MR is in WIP state and it becomes ready (i.e. you did the necessary fixups and rebased on master), please remember to remove the "WIP state".
322
+
TODO: a good example of a merge request with review, discussion, and several iterations is https://gitlab.com/prpl-foundation/prplmesh/prplMesh/-/merge_requests/???
323
323
324
324
### Testing
325
325
326
326
Some changes are riskier than others, since they may break existing functionality which is not yet covered by prplMesh CI.
327
327
Changing the topology handling, channel selection, autoconfig - developers should take extra care when modifying such code areas.
328
328
So, it is expected from every developer to use his/hers good judgement, consult with the EasyMesh Test plan, and run real certification tests
329
329
with real devices using the prplmesh CI infrastructure which allows triggering a test in the CI testbed for a given branch.
330
-
For more information on how to trigger a test, see [Testing in CI](https://github.com/prplfoundation/prplMesh/wiki/Testing-in-CI).
330
+
For more information on how to trigger a test, see [Testing in CI](https://confluence.prplfoundation.org/display/PRPLMESH/Testing+in+CI).
331
331
332
332
### Definition of done
333
333
334
-
Before a pull request can be merged, it must be considered "Done".
334
+
Before a merge request can be merged, it must be considered "Done".
335
335
That means the following conditions must hold.
336
336
337
337
* All commits have a Signed-off-by. Automatic with the DCO check.
0 commit comments