[WIP] feat: propose a protocol for loopback-next PR process #5
Conversation
raymondfeng
changed the title
Signed-off-by: Raymond Feng <[email protected]>
There was a problem hiding this comment.
Great initiative!
Would you like to incorporate some content from https://loopback.io/doc/en/contrib/triaging-pull-requests.html too?
| - CI is green (with exceptions that the failure is caused by known issues) | ||
| - Approved by the owner | ||
| - Approved by mandatory reviewers | ||
|
|
There was a problem hiding this comment.
Let's add also recommended requirements:
- Code coverage is not decreased. Exceptions are allowed after reviewing which lines of code are not covered by tests.
| - Must have/fix (the author must address such comments) | ||
| - Nice to have/fix (the author should try to address such comments but they don't | ||
| block the PR from being landed) | ||
| - Nitpick (up to the author to decide) |
There was a problem hiding this comment.
We could consider adopting Conventional Comments:
- Must have/fix:
suggestion(blocking): <message> - Nice to have/fix:
suggestion(non-blocking): <message> - Nitpick:
nitpick: <message>
| PR will automatically pick up new changes pushed to the same source branch. | ||
|
|
||
| Such commits should be squashed and reorganized to maintain a clean history. | ||
|
|
There was a problem hiding this comment.
Shall we mention a stance against merge commits?
| Merge commits | |
| When iterating on a PR, sometimes it may be necessary to pull in the latest changes from upstream to fix potential merge conflicts. A common approach is to use merge commits. However, LoopBack will not accept Merge Commits as they complicate the Git History and make reviewing more difficult. Instead, `git rebase` should be used to apply the PR commits on top of the latest commit from upstream. |
| community and code matter for us. | ||
|
|
||
| Our maintainers are very distributed around the world with different time zones | ||
| and work schedules. Some of us are paid full-time IBM employees while others are |
There was a problem hiding this comment.
I don't think there's anyone being paid fulltime to manage LoopBack anymore.
| and work schedules. Some of us are paid full-time IBM employees while others are | |
| and work schedules. Most of us are |
| Minimum requirements: | ||
|
|
||
| - CI is green (with exceptions that the failure is caused by known issues) | ||
| - Approved by the owner |
There was a problem hiding this comment.
Perhaps we should give the TSC the ability to act on behalf?
| - Approved by the owner | |
| - Approved by the owner or a TSC member |
There was a problem hiding this comment.
thinking whether it should be TSC member or maintainer.
There was a problem hiding this comment.
I suppose that there's no reason why a maintainer can't approve it. Though I wonder what separates them from the code owner? One thing that comes into mind is being pinged for the PR.
With that consideration, should we formalise a process for deciding who becomes a code owner? This is probably out of scope of this PR, but it may be for future consideration.
There was a problem hiding this comment.
ah. i see. I misunderstood that the "owner" is the PR author, instead of the code owner. I thought code owner is the maintainers of the repo ?
Draft proposal to document/promote our PR process.