Proposed new Curric

New Code Review Curric.md

As a developer, you will very likely work for a company that has a peer review process. While the specific process that a company uses may differ, there is one underlying goal - for you to write better code.

Why code reviews are important?

The code review process has multiple goals:

Ensure the team is staying up to date with changes in the code Ensure everyone is following the company approved style guide Better each others coding skills by creating discussion around different methods, or solutions Ensuring good documentation Catch potential syntax errors or bugs Accountability You and your fellow students will be conducting code reviews for each other, with these similar goals in mind. Not only will this give you a chance to see how other students are building similar projects, it will ensure that all the code you have on GitHub is well organized, neat, and is bug free. This will be crucial when a potential employer is looking through your code. Remember, once they go from your portfolio to your GitHub account, they can and will look at any project they choose.

What makes a good code review.

When you write code, you become attached to it. It's your baby, and the first time you get a code review, it can be scary. It's important to remember that a good code review is not personal. You, or your reviewer, should view the code for what it is, and take the notes as a way of making your code better.

A good code review should identify 4 key things:

potential issues, bad inputs that would break the code, major performance issues syntax consistency issues your opinions on particular statements being unintuitive documentation First, you should identify any issues. If you see a syntax error, or something in the code that should be refactored for performance (it doesn't follow the DRY principle, an algorithm is very slow, or there should be a change for accessability purposes). These things are sometimes harder to spot in other peoples code, because you're not running it, but it's a skill you'll develop.

Second, you're looking for style issues. These are the easiest to spot, by you and a potential employer. You want to make sure that everything is indented consistently throughout each file. Tabs or spaces is irrelevent and a personal choice, but the file should reflect that choice consistently. Quotes in HTML, CSS, and JS are interchangeable but they should also be consistent.

The third thing that you're looking for is "unintuitive statements". Simply that means if you read some code that looks like it's doing way more than it should, or the comment says it should do something, and it's actually doing someting else. Remember, the goal here is to educate each other, so give suggestions and references on where they can read about that different way.

Last thing you should always have in your code, and look for in a review, is documentation. The mantra "Good code speaks for itself" is an excuse not to write documentation. Each of your functions should have a comment above, or inside, that tells the reader what that function does, and what parameters it should expect. Also, you should see comments throughout that help you understand what's happening in the code.

When to get a code review

You're sold on getting your code reviewed, but when should that happen? The best answer is: early and often.

Keep the reviews small (200-400 lines of code). This will make it quick for the reviewer, and quick for you to adjust any notes. Aim for completing a function, feature, or page before getting the review. your reviews will be better, if everything is there. Code review every immediately after commit. One of the great benefits of code review is that it catches certain types of bugs quickly. Also, the fresher the code is in your mind, the quicker you will be able to address the issues that are raised. You will find that getting 10 reviews that are 40 lines each will be easier to get done than 1 review of 400 lines.

GitHub Workflow: Feature, Pull Request, Master

One common method for doing code reviews is to do them on pull requests. You will use this method for the remainder of the course.

When you start a new project, you should create an empty repo, or start with a forked repo. At that point, your master branch is the production ready code, meaning everything there has been reviewed.

The next step is to clone the repo, and immediately switch to a new branch git checkout -b staging for example. Now start coding.

When you're ready for a code review, follow the normal git commit process, and push your code to your staging branch. Now, go to your repo on GitHub, and find the pull request button. You will see a screen that has a dropdown labeled base and one labeled compare. The base is where you want to send the code, so master in this case. The compare is the new code you want to merge in, so staging. Initaite the pull request, and you'll get a screen that gives you a comment box, room to add a title, and a create pull request button. Click that, and you're pull request will be initiated. Now you'll submit the pull request for review by posting it in the #student_code_review slack channel. Once it's reviewed, you can make the adjustments in your local code, and push to GitHub again. This second push will be included in the open pull request.

If you're waiting for a review, you can grab a sandwich, or checkout to a new branch and continue working. If you continue to work on the same branch your pull request is initated from, do not push to GitHub while it's open. This will make your review becoming increasingly longer each time.

Do your first CR.

Now that you undertsand the process of what a good review is, why they're important, and how to get one; it's time to go do one. Head over to the #student_code_review and look for a pull request to review. Grab the most recently posted one, and if it hasn't been merged, do a review. Once you're done, give that link a thumbs up emoji to let everyone else know it's all set. Submit the URL for the pull request you reviewed for the project link at the end of the lesson.

That's it! You made your first review. From here on through the rest of the course, you'll be expected to continue getting your code reviewed and doing code reviews. The only way to get help, is give it.

Focus on the code, not the people. Everyone should remember that the goal of the code review is to improve code quality, not to show off how much better you are than your coworkers. As an author, recognize before the review happens that your code probably has bugs and/or can be improved. This does not make you a bad programmer.

Code Review Checklist

Does the code work? Does it perform its intended function, the logic is correct etc. Is all the code easily understood? Does it conform to consistent code styling? (indentation, quotes, etc) Is there any redundant or duplicate code? Can any global variables be replaced? Do loops have a set length and correct termination conditions? (No infinite loops) Can any logging or debugging code be removed? Do comments exist and describe the intent of the code? Are all functions commented? Is any unusual behavior or edge-case handling described? (is the author handling unexpected user behaviour) Is there any incomplete code? If so, should it be removed or flagged with a suitable marker like ‘TODO’?

Above code review checklist will be applicable for all programming language. But when it comes to specific technology or frameworks it will differ.

jQuery

1. Dom caching should be done
2. Using jQuery Chaining
3. Delegating events via parent
4. Avoid declaring variable inside loop
5. Avoid appending content into dom inside loops.
6. Avoid using .hide, .show , instead use .addClass('hide'), removeClass('hide') (https://twitter.com/paul_irish/status/564443848613847040)
7. Checking hasOwnproperty if for var in loop is used.
8. Using promise instead of callbacks in jQuery ajax.

React

1. Using proptypes for validating props.
2. Using default values for props
3. Prefer self-closing tags for components.
4. Avoiding iteration inside return, move it to render.
5. Unbinding events on componentwillunmount

Angular:

1. Wrapping all code inside IIFE
2. Using controller as
3. Injecting angular dependencies before custom dependencies .
4. Moving reusable logics inside service.
   Note -
   Lots of popular style guide available, we can prepare one for thinkful.

HTML, CSS:

1. Avoiding nesting more than 3 level.
2. Validating the HTML and CSS in W3C
3. Using reset.css
4. Using sprites instead of images.
5. CSS should be placed on top of the page.
6. Scripts should be placed at the bottom of the page.

JavaScript:

1. Feature detection should be done, not Browser detection.

A few typos above -- worth spell checking.

Other than that, i think that providing students with screenshots of an actual code review would be helpful for some of the tips.

Also "You will find that getting 10 reviews that are 40 lines each will be easier to get done than 1 review of 400 lines." -- that may be true on the job, but I wonder if it's going to be true for TF students. Just want to make sure we don't encourage a bottleneck.

Basic checklist...

• Does the code perform its intended function?
• Is the code easily understood? Is documentation needed?
• Are there any agreed coding conventions? If so, does it meet those conventions.
• Is it DRY? Is there any redundant or duplicate code?
• Is the code modular?
• Can you get rid of global variables?
• Is there any commented out code?
• Do loops terminate correctly?
• Can you abstract any unnecessary logic out to helper/utility function?
• Can you remove any of the unnecessary logging/debugging code?

Remember the mantra - I am not my code!