Skip to content

Pull Requests and Code Reviews

Strayker edited this page Oct 29, 2023 · 4 revisions

Pull Requests Creation

Naming Convention: Single-sentence description of changes, capitalised first word, no Conventional Commits/Git Flow signatures.

Target develop for your feature and hotfix branches PRs, main for release branches.

Fill all data of PR's description, based on PR template: Summary - what is happening in this PR, Details - describe changes more briefly.

If your PR is changeing something visual (eg. component on frontend) add screenshot to Summary in description. This technic is helping the reviewer check, if visual changes are working after compilation.

Fill up the following PR's metadata:

  • Reviewer: @StraykerPL,
  • Assignee: yourself,
  • Labels: issue template will add them,
  • Development: connect your ticket here,

Code Review

When you were assigned as a reviewer of some PR, you need to check correctness of changes.

Here is a checklist to follow when performing review:

  • Check if PR is requesting merge to correct branch,
  • Check PR's metadata are filled,
  • Check PR's description to follow guidelines,
  • Check if every CI action is passing,
  • Check changes to follow project's guidelines,
  • Check if all comments are resolved,

This project is following this guidelines:

  • use design patterns where useful,
  • use simple instructions, focus on code readability,
  • prevent code integrity, if something is already done in one way, stick to it or change everything to another way,
  • take under consideration computational complexity and general optimisation, prevent adding not used vars, overworking loops etc.
  • watch for coding standards following,

How to handle PR's Change Requests

After Code Review on PR, there can be comments left by reviewer in GitHub, what must be fixed to merge given PR.

If given comment is provided via review tool in GitHub (tagged as "conversation") this comment must be marked as resolved. First, answer this comment with "Resolved [commit-hash]" on conversation, after pushing commit which is repairing pointed in comment thing. After that, click on "Resolve conversation" to close comment. All comments must be resolved to unlock merge of PR.

Comments that are added to PR's conversation (not via review tool) are not resolvable, so just addressing their requests is enought to do, no extra reference is required.

After resolving comments, click on "re-request review" button in Reviewer section of PR's metadata to inform reviewer, that you finished repairs of PR and this changes are ready for new CR.

Clone this wiki locally