doc: GitHub PRs workflow#8920
Conversation
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## master #8920 +/- ##
==========================================
+ Coverage 82.30% 82.34% +0.04%
==========================================
Files 969 969
Lines 273335 273335
==========================================
+ Hits 224961 225085 +124
+ Misses 48374 48250 -124
Flags with carried forward coverage won't be shown. Click here to find out more. |
jufajardini
left a comment
jufajardini
left a comment
There was a problem hiding this comment.
Looking good, wondering mostly about the timeframe for closing a PR with changes requested, based on the discussion the team last had.
Needs some formatting adjustments, I can work on that bit, but I'll point them out inline, too. :)
| When someone gets assigned a PR, the PR should get a review status within 2 weeks : either changes requested, approved, or assigned to someone else if more expertise is needed. | ||
|
|
||
| GitHub filter for changes-requested PRs is `is:pr is:open draft:false sort:updated-asc review:changes-requested` | ||
| Such a PR may be closed if it has not been updated in the last month. |
There was a problem hiding this comment.
As discussed with the team, isn't a month too short, considering some folks take long vacations?
There was a problem hiding this comment.
Moving to 2 months
| A PR should be marked as `draft` if it is not intended to be merged as is, but is waiting for some sort of feedback. | ||
| The author of the PR should explicit what kind of feedback is expected (CI/QA run, discussion on the code, etc...) | ||
|
|
||
| GitHub filter is `is:pr is:open draft:true sort:updated-asc` |
There was a problem hiding this comment.
requires `` to create inline code:
| GitHub filter is `is:pr is:open draft:true sort:updated-asc` | |
| GitHub filter is ``is:pr is:open draft:true sort:updated-asc`` |
| 1. get reviewed, and either request changes or get approved | ||
| 2. if approved, get staged in a next branch (with other PRs), wait for CI validation (and eventually request changes if CI finds anything) | ||
| 3. get merged and closed |
There was a problem hiding this comment.
These require indentation to be rendered as a list, with one item per line
There was a problem hiding this comment.
So, adding a space at the beginning of each line, right ?
There was a problem hiding this comment.
I usually add a couple, to be safe. Their documentation doesn't mention, this, to be honest, but at least locally I always need that to prevent them from being rendered as simple inline text.
https://www.sphinx-doc.org/en/master/usage/restructuredtext/basics.html
| Draft PRs | ||
| ~~~~~~~~~ | ||
|
|
||
| A PR should be marked as `draft` if it is not intended to be merged as is, but is waiting for some sort of feedback. |
There was a problem hiding this comment.
We are trying to enforce a character limit around 79/80 per line.
| Mergeable PRs | ||
| ~~~~~~~~~~~~~ | ||
|
|
||
| When a Pull Request is intended to be merged as is, the workflow is the following : |
There was a problem hiding this comment.
nit: space before the ":" :P
|
Replaced by #8925 |
2 participants
Link to redmine ticket:
None
Describe changes:
Draft : to be discussed, and I guess after merge of #8898, test if filter for approved PRs works again
Modifies #8913 by putting this in the dev guide