Please, follow the guidelines below when submitting a task code for review.
It is expected that a task codebase lives in its own dedicated repo, and code copy is submitted to this repo for code review purposes. Whenever changes are required these are done in a dedicated task repo, demo gets updated, and then code is copied into this repo fork to update the PR.
A student will benefit from having tasks code base in their own dedicated repos as this forms student's portfolio.
TOC is generated with DocToc
- fork this repository
- clone your fork to your local machine:
git clone https://github.com/YOUR_USERNAME/frontend-2019-p2p.git
- add this repository as an upstream:
git remote add upstream https://github.com/kottans/frontend-2019-p2p.git
- in your local repository, add a folder with your github name under
submissions
In your local repository:
-
git checkout master
-
pull latest changes from upstream
master
branch:git pull upstream master
-
⚠️ create new branch, name it according to task performed (aka feature branch):git checkout -b dom-api-task
. In this example feature branch is calleddom-api-task
-
under your name folder, add a folder with task name (e.g.
js-dom
) and put your code into the folderSee example file structure you are expected to have below:
-
commit your changes to newly created feature branch
-
push feature branch to your remote repository:
git push --set-upstream origin dom-api-task
- open a pull-request from your repository to this repository via GitHub web-interface
- give a PR name according to the task name, also change the PR message as appropriate (follow guidelines in the template)
- make sure the PR doesn't contain irrelevant commits
from your own other PRs or from other contributors.
This may happen if you branch off not from
master
as previous section requires. You may find this Re-sync Fork With Upstream guide helpful to fix the issue.
- you will require approvals from at least two peers, so seek
for sufficient support.
- ask your peers for review in FE Students chat, post a link to your PR as well
- if for some reason there are no reviewers from p2p course available, you may ask your friends from outside the course for a review
- you have to provide reviews on the same task to at least 2 PRs of other students of p2p course once your PR is merged, using your learnings from merged PR code review
- after finishing previous steps ask mentors for review in FE Questionarium chat
- once the code review phase successfully finished and you have approval from other students, post a message in FE Students chat:
{ Here must be the name of your task } — #p2p-pr-done
and add the link to your PR. This step is important, as it helps mentors to track your progress!
Code reviewers are expected to follow code review guidelines.
Before making any changes:
- read code reviewers' comments
- answer questions if any
- explain anything you are asked to explain as this gives an idea on your way of thinking and allows the code reviewer to render an appropriate help
- ask questions if anything in comments or recommendations is not clear
Implementing changes:
master
and also breaks conversation threads
master
into your feature branch, because this will create an additional commit. If you need to updare your branch with files from master, use git rebase
instead.
- when implementing changes according to the changes request, consider cross-checking the entire code base for similar situation and fix as appropriately. Do not expect code reviewers to attract your attention to every single line of code that requires similar fix. Please, take care of your peers for reciprocity
- implement changes in your task dedicated repo and update demo and commit
- copy changed files to this repo fork feature branch and commit
- push feature branch to your remote p2p repo to update your PR:
git push origin dom-api-task
(use actual feature branch name) - supplement commits with messages that give an idea of what's inside of the commit without looking into it. How to Write a Git Commit Message
Go to your PR on github and let code reviewers know your are done:
- resolve conversations you believe you have elaborated on
- make sure you answered questions and gave proper explanations where required (do not resolve those conversations until you have a feedback from a code reviewer)
- if any conversations remain unresolved let reviewers
know your're done explicitly (e.g. write a comment mentioning
a code reviewer using
@
)
PR can be merged by repo maintainers only. When you have required code reviews completed and all code reviewers have approved requested changes, just mention two or three mentors for them to merge approved PR into master
.
Q: Why just pushing a commit with changes is not sufficient?
A: It is not a rare case that changes are introduced in more than one commit. So it is important to let reviewers know they should not expect any further commits. C is for care.