Skip to content

Technical Reviewer's Guide

Michael DiBernardo edited this page Jan 29, 2015 · 5 revisions

This guide describes the motivation and mechanics of technical reviewing for 500 Lines or Less.

What is this book about?

You can read the mission statement in the 500lines repo readme via GitHub. It may evolve as the book grows; we'll let you know if this happens.

What do the contributions look like?

Chapters in this book are structured around describing a short, complete implementation of a working program. You've probably seen the like on your favorite programming blog; the contributor writes up a short program (500 lines max), and then describes it in a sequential narrative, with chunks of the program interspersed throughout.

Some authors may choose to alter this structure, e.g by building multiple versions of a shorter program and describing the evolution at each stage. These cases will be handled by the editorial team, so please rope us in if issues of overall structure pop up unexpectedly.

What is the contribution process and where do reviews fit in?

Here is a rough outline of the contribution process:

  1. Contributor selects a topic and vets it with the editorial team.
  2. Contributor writes a short chunk of their code contribution (~ 100 lines)
  3. Contributor submits this short snippet via pull-request to the 500lines repo
  4. Editorial staff looks over the PR to identify any egregious mutual misunderstandings
  5. Contributor completes "first draft" of max 500 line code contribution, and updates original PR.
  6. Editorial staff will assign a tech reviewer (That means you!) to the PR for a code review.
  7. Reviewer makes comments and suggests changes.
  8. Contributor acts on suggestions by continuing to update original PR.
  9. Reviewer signals they're done with code review on PR.
  10. Editorial staff merges PR.
  11. Contributor begins writing chapter.
  12. Chapter assets and any code changes that pop up during writing are submitted via another PR.
  13. Tech reviewer is assigned to chapter draft (this may be the original reviewer, or a different one. We're not sure yet.)
  14. Similar feedback loop to 7-9 occurs until reviewers, contributor, and editorial staff come to a final consensus on completion.

How do I know what to review?

An editor will reach out to you by email when it's time for you to perform a review. We'll try our best to match you up to something based on your language preferences, area of expertise, or any explicitly stated preferences you may have.

What are the mechanics of performing a review?

Code reviews are performed on GitHub. All technical reviewers will require a GitHub account. If you're not familiar with GitHub, don't worry -- we're not using anything super advanced to do these reviews. If you're familiar with a diff tool and posting comments on conversation threads, that should be enough to do your job :)

Contributors submit code (and often chapters) by forking the 500 Lines repo and sending pull requests. Any changes to these submissions during the initial review process are done by updating the PR, so reviewers can always see a snapshot of the whole contribution by using the Files Changed tab. Individual commits can also commented on.

Reviewers are perfectly welcome to use per-line comments on individual commits in the Commits tab to identify changes / comments, but they should always write up a final summary comment in the Conversation tab which identifies their main concerns.

How do I signal that I think a contribution is good to go?

Post a "Looks good to me", and mention @MichaelDiBernardo in your comment so that he can merge the pull-request. (The mention is optional but greatly appreciated!)

What should I be commenting on during a code review?

Code for 500 Lines or Less is going to be read in book-format by relatively junior developers. As such, we'd like you to help our contributors by commenting on:

  1. Clarity, readability, and consistency of style.

  2. Sequential narrative. Can you read individual modules from top to bottom and wrap your head around what is going on? Don't hesitate to ask the author for hints into how they intend to order discussion of modules in their chapter text if you think that will help.

  3. Are there interesting abstractions in the work? If no, why not? Is the absence of abstractions intentional or accidental? Look for natural conversation points related to abstraction in the piece, and ask the author if they intend to develop / discuss further in their text, as it's possible they didn't even realize. Try to view this as a relatively new developer (e.g. you may shrug off the presence of a singleton or a factory method in your day-to-day work, but a new developer may find these interesting or new.)

  4. Are there natural extension points in the work? Could the author discuss extensibility of a particular module in the text, or even alter the existing contribution to explicity support it?

  5. Is the contribution a straight implementation of a canonical algorithm or pattern? If so, challenge the contributor to consider the "architectural" implications of their piece. Alternately, they could identify points in their contribution where the implementation details of the algorithm require serious consideration (e.g. issues of overflow/underflow not handled by the algorithm statement or the underlying runtime, resource constraints, other noisy real-world problems.)

What should I be commenting on during a chapter review?

  1. Do you feel like there is a 'story' to the chapter? Did that story pull you in?
  2. Is the treatment of the material appropriate to the audience (junior engineers 2-3yrs into a CS degree?)
  3. Are there parts in the chapter where you had to slow down and read something several times to understand it? If so, could the idea be presented differently? (e.g. supplemented with a diagram)
  4. Are concepts and code presented in an order that seems natural to you?
  5. Does the chapter provide sufficient background / supporting material to understand the specific implementation being discussed?
  6. Do you feel like the implementation and explanation differ significantly from how someone would solve this problem "in the real world"? If so, does the text acknowledge this and explain the major differences?
  7. What did you learn from this chapter?
  8. Did you have unanswered questions? What were they?

These questions are not intended to be all-inclusive. If you notice something that you think is worth mentioning, mention it. If you notice that several of your comments are forming a theme that weren't addressed in this section, consider adding it here.

Who should I ask if I have any questions?

Email Michael DiBernardo, or mention @MichaelDiBernardo in your comments on GitHub.