How to Unstick a Pull Request

Sometimes pull requests can get stuck during code review. In many cases, it’s not because the changes were unneeded, but because the conversation just appears to… well, stop.

I’ll walk through five common problems pull requests get trapped in and what you can do as a reviewer to help move things along.

The Forgotten Pull Request

Photo by cottonbro on Pexels.com

Symptoms: There might have been some conversation on the pull request but now it’s just dead silence. Reading back you see that the last comment was from six months ago without clear next steps. The pull request is still open, waiting for someone, anyone, to help nudge it.

Try: Ask the author if the pull request is still needed and if they’re interested in working on the problem. What happens next is either the pull request is swiftly closed OR the author tidies it up for a fresh set of eyes and a new review. ✨

What is the problem? What does this do?

Symptoms: The pull request summary is a terse sentence. In other cases, the pull request doesn’t summarize and has too much context. For whatever the reason, as a PR reviewer you’re not quite sure why the PR exists or how to test it.

Try: There’s no need to be a detective 🕵️‍♀️. If something is confusing, go ahead and ask the author! Being clear on why a PR is needed and what is does, is not only good for the reviewer, but it helps provide context to future maintainers on why certain decisions were made.

Ideally we want to know:

  • What problem(s) the PR is addressing.
  • What does the PR change?
  • How can we manually test it?
  • What type of feedback is the PR author looking for?
  • Are there any trade-offs to the solution we should be aware of?

PR Checks are failing

Symptoms: Newer contributors to a project might not realize that their pull request has failing checks. Maybe the linter is not happy with some whitespace, or a test is broken and the author isn’t sure on how to run the suite.

Try: Gently remind folks in a comment that checks have failed. If you have the time, point to documentation on how to run the tests and other contribution guidelines. It can also be helpful to troubleshoot environment issues with them or give them additional hints on how to clear up the issues. Using our best judgment, we can sometimes accept the PR in a less than perfect state and help fix the issues by asking and making changes on the branch directly, or in a follow up PR.

No Clear Next Steps

Photo by Leah Kelley on Pexels.com

Symptoms: We see a PR where other participants have left some previous reviews. It isn’t clear what needs to happen next. The conversation might even still be active, but it’s going on and on and on and on… to the point where GitHub thinks it’s a good idea to start hiding part of the timeline:

Try: Ask the active participants what needs to happen for the PR to either merge or be closed. It might be as simple as someone taking the time to summarize what was discussed and what needs to happen next, for example: fix tests, address design feedback, and update documentation. At other times, a PR working through multiple concerns might need to be split out: like starting a new PR exploring expanding an API, or proposing a framework change in a different medium like chat or long-form writing.

Additional Reviews Needed

Symptoms: You’re not confidant of approving the PR on your own. The changes affect multiple areas. Maybe the PR needs other expertise like design or security feedback that you’re unable to provide. You’ve done a great job already by knowing what you know and what you don’t know.

Try: Leave a review clearly stating what you tested, what feedback you have, and what type of feedback you think the PR needs to land. Manual testing and other partial reviews are still a great help to other reviewers. If you know who’d be great at unblocking the review, ping them in a comment with context. If you’re not sure, leave a note on what type of reviewer expertise is needed and if you have time, help the author by asking other contributors on who’d be a good fit to review.

Give it a try!

If you notice a stuck pull request that fits one of the patterns here, try unsticking it! You don’t need to be the project expert to help move things along. Simple actions like asking good questions, manually testing, or asking for additional review help can make all the difference.

#code, #process

Remote Communication

Sustainable remote communication requires being intentional and mindful of yours and other’s time.

In distributed companies, we can’t bump into each other in hallways, have casual coffee chats, or even be able to offer a proactive helping hand if a teammate looks confused. Every conversation: be it in video, text chat or private message needs to be initiated by someone with intention.

What we trade for potential increased focus and flexibility, we lose in in-person spontaneity. This isn’t all good or bad, just different. Being mindful of this while we pick the communication tools at our disposal is key to communicating clearly and efficiently.

If you find that you’re constantly task switching, are having endless back-to-back meetings, or are spending all of your time in text chat, this post offers a few things to experiment with and try. 

It’s 2020, so put on your own oxygen mask first 

Before we dive in, I want to acknowledge that there is a pandemic raging, climate change making itself ever present, and all the while, we’re still expected to work. Maybe you’re also struggling with additional caregiver responsibilities, or the social isolation of not being able to go outside and see physically visit people without calculating the risks. 

If this remote stuff feels impossible, know that it isn’t a normal time to be working remotely. Long time remote-first workers are feeling the strain too. Please take care of yourself as best as you can and if you’re a manager reading this post, please encourage folks to take some time off and model that behavior by taking that time off yourself.

Types of Communication Mediums

Long-Form Writing

Long-form writing is the binding glue for most well-functioning remote companies. If you’re ever unsure of a medium to pick it’s hard to go wrong by starting with a long-form post.

A post written by one author can start a conversation in the company with many people and teams. The medium is also flexible enough to handle everything from routine meeting notes and team project questions (working through technical, design or product questions) to a well-honed north-star vision that might guide company direction for the following year.

Benefits

  • Long-form writing forces the author to stop and take the time to frame their thoughts in a succinct and coherent way.
  • Participants don’t need to drop what they’re doing, right now, to focus on a problem that someone raises. They can instead reply when it’s the best time for them. A response within a business day or so, for example, is more than appropriate. This allows for more experimentation with personal schedules. Many find it more efficient to devote one part of their day to reading and responding to posts.
  • It allows folks who are more time-shifted to catch up with what’s going on. This can especially be helpful if you have colleagues in very distant timezones or are working flexible hours.
  • We can eliminate some meetings and better prepare for the remaining ones.
  • With search, we can find out why things have happened in the past without us being there or relying on imperfect human memory.

What type of problems would we pick long-form writing for? Really, just about everything: from a CEO describing company vision to an individual contributor getting into the weeds and needing to work out a solution with feedback from co-workers. 

Examples

  • A group post (written by many people) outlining the north star for company direction 5 years out and a specific roadmap for the quarter. The post asks readers for feedback with a given time deadline, e.g. open for the next 2 weeks.
  • A project lead summarizes the why, the challenges, risks and estimated time for a given project.
  • A designer asks for feedback from the team and stakeholders for the latest flow for their new feature. 
  • A developer outlines two implementation options for the feature and asks for feedback from both product and other developers on tradeoffs and a check in on if they understand requirements.
  • A lead writes up routine team meeting notes: highlighting the agenda, who attended, what was asked, what decisions were made, any follow up action items and who was responsible.

Tool requirements

Roughly, when picking out a specific tool to power internal discussions, it should have the following elements:

  • Posts can notify the right number of people – There should be some ability to tag individuals and larger groups like teams or divisions.
  • Searchable –  letting us find out why things have happened.
  • Editable – Mistakes can be fixed and we can go back to summarize conversations and outcomes at the top of the post, or in a summary comment. For example, if a post asks a question and a direction was picked, we should double-back to edit the post to point out what happened.

Internally at Automattic, we use a tool called P2 to power these discussions. Forum software or wiki tools might also work for you. 

Try this when:

  • You need to collaborate with one or more colleagues.
  • As a first step to see if a problem can be described and next steps can be taken asynchronously, without a meeting.
  • If no other communication medium is a good fit!

Documentation

Documentation is a specialized form of long-form writing where information remains relevant over time and is often maintained by many people.

Tool requirements

  • Searchable –  Information is easily discoverable by anyone in the company
  • Editable – by many people

Examples:

  • Company Culture Book
  • Onboarding documentation
  • Team Processes
  • On-Call Runbook

Try this when:

  • Knowledge is buried in someone’s head and only one person knows how to do something
  • Information is repeated, either through video chats, text chats, or in time-sensitive posts
  • A one-time post resonates with your co-workers more than expected

Workflow Specific Tools (Issues/PRs/Figma/etc)

As a company you may already be communicating asynchronously with very specific tools for a given workflow! For example, bug and feature requests may be written in issues, while developers may spend a lot of time pull requests and code reviews (aka asynchronous pair-programming).

Try this when:

  • Tools and workflows are already in place for your team or company

Try switching mediums when:

  • A discussion is going in circles. Some signs of this might look like needing to click to expand to see a full discussion, or seeing two participants rapidly commenting to each other. When this happens it may be the case that folks are accidentally talking past each other. Text chat or video calls/screensharing can convey information more quickly between two individuals and may help clear up any misunderstandings.
  • We need to step back to better frame a problem at a higher level. For example, instead of going back and forth on implementation details in code, we might need to ask if it’s worth building said feature at all. To reach the right folks, writing in long-form is usually a good call.

If people do choose to switch mediums, please remember to follow up with a summary update or comment summarizing what was decided. This helps others understand what was decided without needing to reach out to others synchronously.

Text Chat

Synchronous communication is a bit of a two edged sword. On the one hand it lets you talk with folks right now, which can help greatly when working through problems and building camaraderie on a team. It also means that we may leave out folks who are in different timezones or are working flex hours out of conversations.

For some, text chat can be very distracting and draining. Let’s go over some reasons why this might be so.

  • Are you allowed to mute chat for focus time, or after hours?
  • What is the expected response-time for a ping in a public channel or direct message? Talk with your lead to get a clear understanding! For example, since Automattic has no core working hours, on my team it’s okay to respond within a business day if folks are not online or have muted chat for focus time.
  • If you’re not on-call, how well are you silencing these notifications on personal devices after work?
  • Can private messages be taken to a more public channel instead? Too many private messages can feel like the equivalent of someone stopping by your desk and interrupting you. If the subject isn’t confidential and can be discussed in the open (think projects not feedback), consider moving the conversation to a more public area. This helps gives others visibility of what you are spending your time on. It also avoids ever growing group direct messages where some subset of folks have half of the conversation.
  • If someone asks for non-urgent help and you’re in the middle of something, can you schedule a time to chat in text or video later on the calendar?
  • Don’t force others to read text chat backscroll. If an interesting conversation has occurred, and we’d like to share what was talked about, a long form post summarizing this is much more appropriate than having someone else parse a long-winding conversation.

Try this when:

  • You need a bit more bandwidth to quickly discuss an idea with a few people.
  • Team/co-worker bonding! a.k.a all of the emojis and gifs! ✨

Video Chat/Screen Share

Video chats and screen sharing demand one’s immediate attention. I can juggle a few ongoing text chats, but I can manage only one video call at a time. In return for this demand on our attention, we can share a lot of information in a short period of time.

Try this when:

  • We’re building relationships! 1:1s, the weekly team call, watercooler chats and so on!
  • When we quickly need to build consensus or brainstorm next tasks in meetings.

Video calls, for whatever reason, tend to be more draining than meetings done in person. Except for 1:1s, aim to keep meetings small, short and on point. An hour for example tends to be the maximum limit of focus, no matter how much people would like to pay attention. Team process may need to be altered to help accommodate this additional cost. I’d recommend simplifying process as much as possible, and preferring async over sync when looking at communication options.

Summary

Regardless of what medium we pick, remember to communicate regularly and often! While working remotely, we often need to work twice as hard to make sure folks know what we’re accomplishing and when we need help.

Communicating in a sustainable way requires being intentional and mindful of yours and other’s time. If this ever feels draining, try switching mediums or start a discussion with your lead on experimenting further with process. It may make all the difference!

#process

Code Reviews 📚

The following is a collection of my thoughts about what makes a good code review. This is a repost from the internal Calypso blog with a few modifications made from feedback. I have also included a few tips to structure PRs in a reviewer friendly way. It’s my hope that this post will help encourage folks to get excited about code reviews.

Frame of Mind

At the start of my career, I didn’t understand why good code reviews were helpful. This was partly because I hadn’t seen a good code review yet. At best someone might have rubber stamped my change, and at worst code gatekeepers nitpicked irrelevant details in the patch leaving both parties in a foul mood.

My thoughts on reviewing changed drastically once I realized I was approaching this with the wrong frame of mind. We are rewarded by what effort we put into it, and as part of that participants must share some common understandings to avoid an unproductive review.

At Automattic, I think we already have a very strong reviewing culture. The following are a few points I personally remind myself, before starting a review.

We all share ownership of the code. It is not yours, or mine, it’s ours. Always welcome improvements and share knowledge freely.

Many programing decisions are opinions. There is often no one right answer. Discuss tradeoffs in a productive way and move on quickly. Stick with project convention for stylistic things like tabs vs spaces, even if you don’t agree with it. Changing convention can be done outside of the PR as a larger discussion with the group.

We can always learn new things. No matter how much one knows, we can always learn more. Folks can expose you to new ideas. Explaining concepts you’re familiar with can help improve your understanding of it.

Communicating

Another huge part of a successful code review is good communication. We’re all nice folks at Automattic, but text communication is tricky. It is very easy to misinterpret feedback about code as something more personal. I think we’ve done a very good job at avoiding this, but here are a few techniques to lesson confusion.

  • Avoid separating code ownership. Do not assign ownership of the code with words like “my code” or “your code”. Doing so makes code reviews feel more like a personal judgement. We all share ownership of the code. Remember that the code is also a product of many constraints (time, familiarity with the codebase, etc.) and is not a personal reflection about the author. Even the best developers will produce code from time to time that has some issues to work through.
  • Assume best intent, stay positive. Avoid sarcasm and negative descriptors like “terrible” or “dumb” that may be misread.
  • Avoid demands, offer suggestions instead. “What about moving this into it’s own file?” It’s also helpful to phrase these as questions. Often times the reviewer may be missing context on why a particular suggestion will not work.
  • Authors should respond to suggestions. “Great catch! Updated in 565acae.” “We went ahead with the original approach because of timing concerns.”
  • Be explicit. “Let’s do change X because of reason Y”
  • Say if something is a blocker or optional. “Due to security concerns we should update this method before shipping.” “This is optional but I think this reads better if we move this into it’s own method.”
  • If something is confusing, ask. “What is the reasoning behind these changes?”  “I don’t understand what’s happening on this line, could you please explain?
  • Let the author know when you appreciate a change. “Thanks for taking on this task!” “I really ❤️ how this new workflow feels, I left a few notes on some things we can improve.” “This PR drops our build size by 500kb! Great work!”
  • Explain next steps, or complete the review. “I noted a few blockers I’d like to see resolved before we 🚢” “Changes here look great. 🚢 when you’re ready.”
  • Keep up momentum. If a PR looks stalled ask if anything needs to be done. This is especially important for OSS contributors. It is usually better to accept a PR that has a few issues left to work through, and fix it up later, than have the OSS person abandon the PR.

Preparing your PR to be Reviewed

If folks are always waiting for a code review it helps to have some empathy for the reviewer too!

  • Explain why. Assume reviewers have little or no context when reading the PR. Explain why we need this PR and what it does. (This is also very useful when looking at past decisions). Screenshots and gifs are appreciated when behavior is complex.
  • Add Step-By-Step Test Instructions. Can someone unfamiliar with the changes test your PR by reading the summary?
  • Keep changes small. Large changes are difficult to review and understand. Try to separate janitorial changes from PRs that change behavior.
  • Note weird things. This includes explaining any odd code workarounds, or buggy behavior. This can save some back and forth between the reviewer and author, and may also expose existing bugs.

Code Review Benefits

When done well, code reviews can help on many different levels.

  • It spreads code ownership.
  • Communicates changes across teams.
  • Serves as a sanity check to verify that requirements make sense.
  • Allows folks to find bugs though both manual testing and in code.
  • Lets all folks involved learn new things!
  • Can also serve as documentation for how something worked, and why certain decisions were made. Perhaps even for a future you!

Anyone can be a Reviewer!

The fact that code reviews work on many levels also means that reviewers don’t need to know all things about a project in order to make a meaningful contribution. Sharpening copy, manually testing, polishing design, or asking questions about confusing things is a great help.

Code Review Challenge

If this isn’t a habit for you yet, I’ll like to challenge you to try reviewing a few PRs from a different team or one that you may have felt intimidated to contribute to.

Here are a few strategies I use to pick PRs to review:

  1. Take a look at one of the oldest PRs on the needs review list. Ping the author with questions if it looks inactive.
  2. If you don’t have a lot of time, choose a tiny PR to look at. These are the fastest to review and test, and usually have the least risk of causing a regression.
  3. Choose something you’re unfamiliar with. Reviewing PRs is a great way to learn, and to keep a pulse of what’s happening on other teams. Don’t be afraid to dive into a section you’ve never looked at before. If you don’t follow, or something is unclear, ask questions! The PR author is usually happy to explain.

Have fun reviewing!

#code, #process

Work and other things

I work for Automattic a completely 100% distributed company with employees from all around the world. Once a year we hold the Grand Meetup, a frenetic week long event where the whole company flies in to blanket an unsuspecting off-season ski resort with our own wifi and friendly Automatticians.

It’s a crazy-fun experience, and one of the rare chances we get to see the folks we work with in person.

team

My awesome team

 

While at the meetup, I got a lot of enthusiastic thanks from folks for helping out on reviewing their pull requests. It was extremely flattering, but I also had the nagging thought of “Is it so unusual for cross-team reviews to happen?”

So Many Pull Requests

stack.gif

To give some more context, I primarily work on Calypso, an open source dashboard for reading, writing, and managing all of your WordPress sites in one place.

At any given time there are roughly 3-5 pages worth of PRs with a needs review label. This is pretty overwhelming. I can understand how easy it could be to have tunnel vision and only keep track of your teammate’s work.

To encourage folks to review a bit more, here’s a quick summary of how I break up my day:

Morning – review all the things!

I happen to work in the latest time zone on my team. This means that by the time I’m online, the rest of my team already has a number of in-progress or needs review PRs that are fresh and waiting for me. This is one of my only chances to chat with folks while they’re also awake, which makes it a great time to catch up with folks or have a live video hangout.

My morning time is very interrupt-driven, which is why this is also the best time of day for me to review my team’s PRs. I have a special email filter for Calypso pings. I try to run an inbox-zero approach here, but depending on current project needs I might not be able to get through all of them.

coffee.gif

Coffee and Breakfast is also very important

 

Afternoon – Coding

By the time I’ve finished lunch, the rest of my team should hopefully be relaxing as their day winds down. Slack is mostly quiet in my afternoons, aside from helping out with a random Calypso fire or two that may occur from time to time. I usually have a solid chunk of uninterrupted time, which is great for getting into the flow, loading a complex problem into your brain, and cranking out a decent approach to a problem.

I stop when I’ve hit a good point, where I’m pretty certain I won’t be dreaming about solving the issue in my sleep.

code.gif

Evening – Trash Pickup or Cross Team Reviews

After hitting a good point with my in progress issue, I usually have a little bit of time left in my day. To wind down, I might pick out a quick janitorial issue to hammer out, or more often I might peruse that giant list of PRs that need to be reviewed:

giphy-1.gif

But how to pick one?

Picking a PR to Review:

Here are some suggested approaches:

  • Take a look at one of the oldest PRs on the list. Ping the author with questions if it looks inactive.
  • If you don’t have a lot of time, choose a tiny PR to look at. These are the fastest to review and test, and usually have the least risk of causing a regression.
  • Choose something you’re unfamiliar with. Reviewing PRs is a great way to learn, and to keep a pulse of what’s happening on other teams. Don’t be afraid to dive into a section you’ve never looked at before. If you don’t follow, or something is unclear, ask questions! The PR author is usually happy to explain.

Always Waiting for a PR Review?

Do you always feel like you’re waiting for a review? A few tips:

  • Add context and reasons of what your PR is trying to achieve in the summary, so someone unfamiliar with the issues can hop in and help.
  • Add step-by-step testing instructions! This makes it easy for people to review your code.
  • Break down huge PRs into manageable chunks. Large diffs are incredibly hard to review and test, and it’s more likely that folks just don’t have that amount of time to sit down and attempt to understand the PR.
    • The might mean adding wip PRs with feature flags
    • Sorting out janitorial things, like updating code-styles into their own PRs
    • Do things in steps. For example, first creating a PR to add a new component, before using it on a page in a second PR
  • Take some time to review a few PRs! Other folks are waiting too!

Night – Relax!

Your day might look very different, but I find that interrupted time is great for PRs, and quiet time is incredible for coding.

Remember to take it easy though and not to overwork yourself. I personally try to work somewhat traditional hours and turn on a do-not-disturb mode on my phone for night time hours. This helps me from from getting sucked back into work mode too late at night. Other folks might be able to handle a much looser schedule, but I am not one of them. 🙂

Anyway, I hope this post was helpful! To my co-workers it was great seeing you at the Grand Meetup, and I can’t wait to see you all again!

bear.jpg

 

#process

s
search
c
compose new post
r
reply
e
edit
t
go to top
j
go to the next post or comment
k
go to the previous post or comment
o
toggle comment visibility
esc
cancel edit post or comment
%d bloggers like this: