PR supervision
parent
d144c6d263
commit
4a2c1cf770
|
@ -1,14 +1,11 @@
|
|||
“Reviews allow for discussion of proposed changes and help ensure that the changes meet the repository’s contributing guidelines and other quality standards.“ (from GitHub documentation)
|
||||
|
||||
## How do I process own PRs
|
||||
## General principles
|
||||
|
||||
* Decide if my PR is (a) ready to review or (b) I just need to complex testing of my contribution or (c) I want to share my work-in-progress and ask someone for help:
|
||||
- (a) -> make it as regular PR
|
||||
- (b,c) -> make it as Draft PR
|
||||
* Make PR title reasonably short and sufficiently self-explainable so just after reading it anyone can get a basic idea of what it is about.
|
||||
* Write short summary of your work, [link issue](https://docs.github.com/en/free-pro-team@latest/github/managing-your-work-on-github/linking-a-pull-request-to-an-issue) which it fixes or closes, eventually what is missing to be done as a checkpoint-list.
|
||||
* Add a label to classify the PR if it is fix or enhancement, logger to another part of code so reviewers can simpler take it according to their domain of expertise...
|
||||
* Add milestone where it shall fit so we can put it to timeline, mainly distinguish if it goes to closes bugfix release 1.0.x or feature release 1.x.0
|
||||
* **We value everyone putting time and effort into PyTorch Lightning, but not every change should be included.**
|
||||
* We encourage everyone to open issues describing what they want to do before going off and doing it.
|
||||
* Every feature we add takes attention and maintenance, but most importantly- might break something else.
|
||||
* Discuss the tradeoffs in the PR or associated issues.
|
||||
|
||||
### Feature vs bugfix
|
||||
|
||||
|
@ -20,12 +17,24 @@ Each of them has its own label and accordingly, they shall be put into a milesto
|
|||
- update behaviour of a function to work as expected/desired
|
||||
- fix crashes with expected input arguments
|
||||
|
||||
## General principles
|
||||
## How do I process own PRs
|
||||
|
||||
* **We value everyone putting time and effort into PyTorch Lightning, but not every change should be included.**
|
||||
* We encourage everyone to open issues describing what they want to do before going off and doing it.
|
||||
* Every feature we add takes attention and maintenance, but most importantly- might break something else.
|
||||
* Discuss the tradeoffs in the PR or associated issue.
|
||||
* Decide if my PR is (a) ready to review or (b) I just need to complex testing of my contribution or (c) I want to share my work-in-progress and ask someone for help:
|
||||
- (a) -> make it as regular PR
|
||||
- (b,c) -> make it as Draft PR
|
||||
* Make PR title reasonably short and sufficiently self-explainable so just after reading it anyone can get a basic idea of what it is about.
|
||||
* Write short summary of your work, [link issue](https://docs.github.com/en/free-pro-team@latest/github/managing-your-work-on-github/linking-a-pull-request-to-an-issue) which it fixes or closes, eventually what is missing to be done as a checkpoint-list.
|
||||
* Add a label to classify the PR if it is fix or enhancement, logger to another part of code so reviewers can simpler take it according to their domain of expertise...
|
||||
* Add milestone where it shall fit so we can put it to timeline, mainly distinguish if it goes to closes bugfix release 1.0.x or feature release 1.x.0
|
||||
|
||||
## PR supervision
|
||||
|
||||
We have observed that we have some long hanging or abundant PRs. Let's get them done all - they shall land on Pl or get rejected for a reason (the feature/bugfix was implemented already in another PR or they are very outdated and it would be much easier to create a new one).
|
||||
To reach such flow, all PR which are not authored by core-contributor, shall be assigned to one core-contributor who will take care of it, in particular:
|
||||
- decide if this PR shall land or be rejected
|
||||
- taking close look and mentor PR author
|
||||
- help if partial fixes requested by reviewers if needed
|
||||
- keep the PR actively in progress till it lands :]
|
||||
|
||||
## How do I review a PR?
|
||||
|
||||
|
@ -57,7 +66,7 @@ Each of them has its own label and accordingly, they shall be put into a milesto
|
|||
* Make sure error scenarios and edge cases are well handled.
|
||||
- Would this work for a single node? multi-node?
|
||||
- CPU? GPU? TPU?
|
||||
- Anything with spawn needs to be serializable. _Spawn is basically like: a car is driving down a single lane- this is the main process. When spawn is called, that car gets copied into 8 other cars (8 GPUs). That car then goes into its own lane. The car is trained on that lane, but the original car isn’t... so consider what happens in those scenarios._
|
||||
- Anything with spawn needs to be serializable. _Spawn is basically like: a car is driving down a single lane- this is the main process. When the spawn is called, that car gets copied into 8 other cars (8 GPUs). That car then goes into its own lane. The car is trained on that lane, but the original car isn’t... so consider what happens in those scenarios._
|
||||
- _DDP is like the same analogy but instead of copying the car 8 times, we copy it 7 times and the main car then joins those 7 to train like all._
|
||||
- Make sure this works with 16-bit precision.
|
||||
|
||||
|
@ -68,7 +77,7 @@ Each of them has its own label and accordingly, they shall be put into a milesto
|
|||
|
||||
### 6. Ready to go :]
|
||||
- If you as a reviewer find the PR fine, and all tests are passing, you can place the label "Ready-to-go".
|
||||
- This "Ready-to-go" labels shall not be used by the PR author 8-]
|
||||
- These "Ready-to-go" labels shall not be used by the PR author 8-]
|
||||
|
||||
## Approving PRs
|
||||
|
||||
|
@ -83,5 +92,5 @@ Remember that if you approve a PR and it reaches the minimum number of required
|
|||
|
||||
## Reviewers are our friends and partners!
|
||||
|
||||
- **Be nice and constructive**, remember your fist PR and what helped you / made you motivated to continue contributing.
|
||||
- **Be nice and constructive**, remember your first PR and what helped you / made you motivated to continue contributing.
|
||||
- **Use suggestions** for simple edits when you know what it should be instead of leaving a long description (you can still add a short justification of your proposal) so the author can simply accept it.
|
||||
|
|
Loading…
Reference in New Issue