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/ @doctocat

Labels

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

Sympathy for the reviewer

By kennyadsl

Sympathy for the reviewer

  • 707