Skip to content

Instantly share code, notes, and snippets.

@ViBiOh

ViBiOh/Github untold manual.md

Last active August 27, 2024 08:27
Show Gist options
  • Save ViBiOh/1f2a4f4cdee5a343d2e31a2204f137a1 to your computer and use it in GitHub Desktop.
Save ViBiOh/1f2a4f4cdee5a343d2e31a2204f137a1 to your computer and use it in GitHub Desktop.
Download ZIP
Raw
Github untold manual.md

Github Pull-Request untold manual

The Team

  • Coding is work. Reviewing is, also, work. The added value of the code resides in its usage. Code is used when merged, merged when reviewed. So keep in mind that reviewing is adding value.

  • Every member of the team should have at least one dedicated time slot (two or more is more healthy) during his regular working day to perform code reviews. For example, at the beginning of the day, before starting something else, take a tour of GitHub. In the same way, when coming back from lunch.

  • To know what is waiting for a review, the Review Request can be helpful (but sometimes too noisy). Take time to configure your Notifications, it worths it.

  • Even if you are busy working on the next big thing, don't let others waiting for you, ensure that everyone can work at their full potential.

  • The team owns severals projects and the whole team is notified when a pull-request is opened. Once a review has been started by someone, it disappears from the "Review Request" tab. So, once your start a review, you are responsible to review it once merged or closed.

  • There is a GitHub feature for load-balancing review between the member of the team, but it requires some discipline on each member, declaring your vacations. It should be used at last if we fail to organize by ourselves.

Notification

  • GitHub has a built-in notification system which can be powerful if used properly.

  • If you're not fan of GitHub UI and prefer having all in Slack, you can configure Scheduled Reminders to receive it where you want.

  • If you're not fan of GitHub UI and prefer receiving emails, you can also configure Email Routing alongside with Notifications

The Developer

  • A pull-request is about proposing to change code (adding, removing, updating), in an asynchronous way: be verbose. Explain why, what and how you change things, if the code is not self-describing.

  • Before asking someone to review your code, review yourself. The debug print, the temporary value, the TODO are things that happen.

  • Your commit history should tell a story. Even if they are squashed after, during the review, it helps your reviewer to understand your path to the given changes.

  • You can add more semantic to your commit by following some conventions like Conventional Commit, GitMoji. Whatever you prefer, it's always better than "adding feature", "working stuff", "quick fix", "feedback fixes".

  • When you push new changes to a pull-request which review has started, it notifies The Reviewer. Turn your pull-request into a draft if there is still ongoing work.

  • Ensure to push code that comply formatters, linters, tests, etc. Test your code locally. Value time of others. Don't let the Continuous Integration tell you that formatting is wrong, you can fix it before. Don't let The Reviewer find linter or compilation mistakes.

  • Don't mix things. It's already hard for The Reviewer to jump on an unkown topic, don't add multiples topics. Separate in commits, separate in pull-requests, based on the size of changes.

  • Don't stack your work, if you have more 5 pull-requests opened (this number is arbitrary, everyone has its own), kindly remind to The Reviewer that you are stucked. Stop starting, start finishing.

  • You are not the code you write. If someone reviews your code, it's the code that is reviewed, not you. Easy to say, not easy to do, but don't make it personnal, it shouldn't. In the end, the code belongs to the team.

  • The goal of the review is to ensure quality and consistency of the codebase for the repository, the team, etc.

  • Don’t resolve conversation yourself. The Reviewer asks for something, that’s his responsibility to mark the conversation as resolved, based on your modification / answer. It's easier to follow.

  • Once the review has started, avoid pushing force on the branch. GitHub has a feature "New changes since you last viewed" that help The Reviewer to read only new changes instead of the whole pull-request.

view_changes

  • Unless your changes are critical (ongoing incident, security issue, etc) you shouldn't have to beg for a review on a corporate chat. Someone will pick it sooner or later. Everyone is working, in an asynchronous way and does his best, patience is key. After one full day, if no one have picked your pull-request, a kind reminder to the process can be necessary 🤗

The Reviewer

  • We should trust each others as equal in the review process. Unless a specific member of the team is mentionned in the Pull-Request description, anyone should feel comfortable to start a pull-request review.

  • When you don't know, you don't know, don't be afraid to ask for help by mentionning people.

  • Do not assume people know or have bad intents, ask genuinely and keep a learning/teaching mindset.

  • If multiple people go on the pull-request at the same time, the Review workflow is not efficient. Assign yourself before starting the review. It lets other know that someone is on it.

assign yourself

  • Review the code, according to the context given by The Developer. Don't review The Developer. Ask yourself "in 6 months or more, will it be understandable?". Code is read 10 times more than written

Some interesting articles

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment