Open illicitonion opened 2 years ago
Notes from further discussions:
I like the general idea! 👍🏻 A couple of thoughts:
From discussion on slack, some action items:
For future reference, here's the draft we're working against: https://github.com/illicitonion/JS1-Code-Review-Draft
Aiming to have this done by LDN9 JS1W4 (prob Jan?)
Which module(s) and week(s) does this change affect? Module(s): JS1
Goals
Idea
Each week, pair up trainees randomly, and give them each two exercises. For each exercise, one trainee will be the reviewer, and one the reviewee. Each trainee should do both roles for the two exercises in the week.
Both will be given some code which has problems in it.
The reviewer will additionally be given a list of problems, with explanations of what's wrong, and examples of better code.
The goal of the reviewer is to make comments on a pull request to guide the reviewee to improve the code. We expect this to take place over multiple iterations over a week, as part of coursework.
Ideally the material being reviewed should be material the trainees are fairly comfortable with, and reinforce, but should not be pushing their knowledge too far - we want the trainees to be able to mostly focus on the code review aspects, and not get bogged down in one or both of their pair not well understanding the code itself.
Rules of the game
We need rules for the reviewer, to avoid them just giving replacement code. Something along the lines of:
Suggested subject matter for each week
Week 1: HTML/CSS (e.g. adding/improving aria labels, factoring out common CSS into common rules, removing redundant CSS) Week 2: Variable / parameter naming, duplicate code Week 3: Loops, boundary conditions, more naming Week 4: Choosing between for loops/forEach/map, pulling out chains.
Throughout, but particularly in week 4, we should make clear how to escalate for more help if the trainees find themselves out of their depth, probably by @-ing a mentor to help out. The same rules of the game should apply to mentors as to trainees.
Open questions
Who might need to know about this change?
@CodeYourFuture/syllabus-team - I'd love to talk this through in our meeting this week, I'll try to put together some example code before then.