| Previous Lecture | Lecture 20 | Next Lecture |
Lecture 20, Wed 05/12
Wed Lecture:
Pull Requests
A few things we didn’t say yesterday, but are important to know:
When you are done with your issue, and make a Pull Request:
- Link the PR to the issue, and/or the issue to the PR. (Once you make the link, it exists in both directions)
- Make sure the PR is tagged with your team’s tag, e.g.
s21-5pm-1,s21-5pm-2, etc. - Make sure to add PR description. It should briefly describe the change you made from the user perspective.
- Assign yourselves on the PR (the same folks that worked on the issue.)
- Ask for code reviews from:
- Members of your team that didn’t work on the issue (see team list here)
- From the specific LA and TA that are assigned to your team (see team list here)
- Don’t ask for code reviews from “just everyone” on the staff (though other staff members can do PRs and will sometimes)
- It can be helpful to deploy the branch on your sites Heroku deployment (the one from the team03 project)
You may see tags such as these:
| Tag | Explanation/What you need to do |
|---|---|
FIX ME-MISSING TEAM CR |
Get a fellow member of your team to do a code review |
FIX ME-TEST COVERAGE |
Tests may be passing, but they are too many missed lines (i.e. lines not covered by tests). Improve the test coverage (ask staff for help if you aren’t sure how) |
FIX ME-CODE REVIEW ISSUES |
Someone requested changes in the Code Review, but you haven’t addressed them yet |
FIX ME-PR Description |
Your PR doesn’t have a description, or the description needs additional attention |
How to do a peer Code Review
There are two aspects to doing a peer code review:
- Getting the mechanics right (i.e. clicking on the right buttons in the right way)
- Providing meaningful review and feedback (to help your team improve and keep the quality of the code base high)
Both are important, and arguably the second one is more important. But, let’s tackle the first one first.
Doing a code review isn’t just making a comment such as LGTM on the PR. That’s helpful, but it doesn’t trigger the mechanisms in GitHub’s webapp to mark the PR as having been code reviewed. That requires a specific series of actions on the GitHub web site, done in a particular way.
We’ll be putting up a YouTube video explaining that will be published shortly as part of this series:
| Step | Explanation | Video Link | Starting Kanban Board Column |
Ending Kanban Board Column |
|---|---|---|---|---|
| 1 | Have idea, create issue, groom issue | https://youtu.be/IzMml6aFvRY (8:32) | Planning | ToDo |
| 2 | Assign issue to self (and possibly pair partner) | https://youtu.be/YMDldDQ3e54 (3:46) | ToDo | InProgress |
| 3 | Work on Issue | https://youtu.be/DbaBdl1TUuk (7:06) | InProgress | InProgress |
| 4 | Create PR | https://youtu.be/TKj9AwP10qQ (TBD) | InProgress | InReview |
| 5 | Code Review PR / Testing | Coming Soon | In Review | In Review |
| 6 | Merging | Coming Soon | In Review | Done |
But in the meantime, I’ll give a short live demo turing today’s class.
The short version:
- Don’t just make a comment
- Use the “Files Changed” tab, and the green
Review Changesbutton at upper right.