Sympathy for the reviewer
Alberto Vena
kennyadsl
"Matz is nice
and so we are nice,"
But... are we nice with who reviews our code?
Who reads our PR?
- Code owners: team members, team lead, who is in charge of reviewing code and/or merge it
- Who adds features: When adding new code we are asked to understand the context of existing code
- Who fixes bugs: often, a bug related to existing code is not fixed by the person who introduced it
A Pull Request is created one time but evaluated multiple times.
myself, cit.
Code
Code
What can we do when coding?
- Split feature in smaller PRs
- Small commits well documented
- Tests (help with context)
- Code documentation (YARD, etc)
- CONTRIBUTING.md (Style Guide, etc)
Documentation
TITLE
Describe what PR does
"could this title fit a changelog entry?"
Description
Further explains what changed,
but also focuses on why.
A good description has:
- Quick recap: what changed
- Why things changed
- Steps to reproduce/test
- Screenshots
- Checklist
- added test
- QA approved
PR Templates
Enforces things that matter for the project.
.github/PULL_REQUEST_TEMPLATE.md
Resources:
**Description**
<!--
Please include a summary of the change and which
issue is fixed. Please also include relevant
motivation and context.
If needed you can reference another PR or
issue here, e.g.:
Ref #ISSUE
-->
**Checklist:**
- [ ] I have followed [Pull Request guidelines](https://github.com/solidusio/solidus/blob/master/CONTRIBUTING.md#pull-request-guidelines)
- [ ] I have added a detailed description into each commit message
- [ ] I have updated Guides and README accordingly to this change (if needed)
- [ ] I have added tests to cover this change (if needed)
Multiple Templates
.github/PULL_REQUEST_TEMPLATE.md
+
.github/PULL_REQUEST_TEMPLATE/infrastructure.md
Assign reviewers
Code owners
.github/CODEOWNERS
# These owners will be the default owners for everything in
# the repo. Unless a later match takes precedence,
# @global-owner1 and @global-owner2 will be requested for
# review when someone opens a pull request.
* @global-owner1 @global-owner2
# Order is important; the last matching pattern takes the most
# precedence. When someone opens a pull request that only
# modifies JS files, only @js-owner and not the global
# owner(s) will be requested for a review.
*.js @js-owner
# You can also use email addresses if you prefer. They'll be
# used to look up users just like we do for commit author
# emails.
*.go docs@example.com
# In this example, @doctocat owns any files in the build/logs
# directory at the root of the repository and any of its
# subdirectories.
/build/logs/ @doctocatLabels
often unused
Staging
For
each
Pull Request
Preview APPS
curl -X POST -H "Content-Type: application/json" \
-H "Accept: application/vnd.github.ant-man-preview+json" \
"https://api.github.com/repos/nebulab/floyd/deployments/42/statuses?access_token=XXX" \
-d '{ "state": "success", "environment_url": "https://your-custom-url/", "environment": "preview"'Preview APPS
In Nebulab, we use a CircleCI Orb to share the Preview Apps configuration among our projects.
It's open source here:
version: 2.1
orbs:
feature-branch: nebulab/feature-branch-preview
workflows:
cool-workflow:
jobs:
- do-cool-things
- run-specs
- feature-branch/preview:
domain: <domain-for-preview-apps>
github_repo: <your-github-repository>
github_token: <your-gh-auth-token>
letsencrypt_email: <email-for-ssl-cert>
server: <docker-server-to-run-containers>
user: <docker-server-user>I don't want to be nice,
WHY Should I care?
- Better (contextualized) reviews -> Code quality
- Faster reviews -> Improved velocity
- Better development of new features
- Easier debugging
Am i Missing something?
Please help me with my blog post 🙏
Thanks
Alberto Vena
kennyadsl