-
Notifications
You must be signed in to change notification settings - Fork 155
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
docs: add code review guide #2772
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thank you @hairyhum !
CODE_REVIEW.md
Outdated
- If PR changes documented behaviour, there should be update to `/docs` or inline comments | ||
- Reviewer should request changes from contributor to update docs | ||
- If PR introduces breaking changes, fixes a bug or adds a new feature, there should be a [release note](#release-notes) | ||
- Reviewer may add a release note by themself |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should clarify, whose responsibility it is to add the release note. I think it's PR author's responsibility. if we not explicitly set the expectation here, one would think the other is going to do that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point. Added to contributors guide and mentioned here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
81ff51e
to
829dd7c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like we are missing new lines at the end of some of the files. otherwise looks good.
Change Overview
Added new document with instructions on reviewing the PRs.
This guide mentions RENO release note tools from #2604
Pull request type
Please check the type of change your PR introduces:
Test Plan