Practice Code Review

Before jumping into group projects using JS, take some time to learn about code review and code quality automation . Practicing with these concepts now will make the transition to collaborative development much smoother.

• Big Picture
  • The Challenges
  • Planning
  • Code Review
• Writing Code
  • Repo Setup
  • Local Setup
  • Developing Your Solutions
• Code Quality Automation
  • npm scripts
  • Code Coverage
  • Continuous Integration

---

The Challenges

Your group's challenge is to write as many strategies and implementations as you can! Code quality should be your main focus. You'll practice formatting, linting, testing, and code review.

There are already a few challenges in the /src folder, but feel free to add a new one if there's a different problem you'd like to solve. And it's ok if someone has already tried using the same strategy to solve the same problem you want to work on. You can always find a slightly different approach, and comparing your solutions will be a good topic for the retrospective.

As usual, it's more important to write great solutions to easy problems than to write ok solutions to hard challenges. So take it easy and have some fun.

Template

./.template-solution

/practice-code-review
  /.template-solution
    /README.md
    /sandbox.js
    /solution-name.js
    /solution-name.spec.js

what are all these files?

• /README.md:
  • Challenge Description: A short description of the challenge.
  • Strategy: Describe the strategy you used to solve this challenge. Why did you choose this strategy? What did you learn from using this strategy?
  • Implementation: Describe your implementation. How did you implement your solution? What language features did you use and why?
  • Use Cases: Include one or two use cases for how your function could be helpful in the real world.
  • Inspiration: Which classmates, code, blogs, or videos inspired you? Include links and explain what you used from each source.
• /sandbox.js: An empty file just for you. You can import your solution and write whatever experiments you like in here, it will not be reviewed when you send your PR.
• /solution-name.js: Your function will be exported from this file to the .spec.js and sandbox.js files. solution-name.js will contain only two things:
  1. your function's JSDoc comment
  2. your function
• /solution-name.spec.js: Unit tests for your function, nothing else. This file will import your solution and test it. There will be no extra logs, experiments, or comments in this file.

Example Solution

./src/sort-numbers/example-built-in-sort

/practice-code-review
  /src
    /challenge
      /strategy
        /README.md
        /sandbox.js - for experimenting and logging
        /solution-name.js - for your function
        /solution-name.spec.js - for your tests

---

Planning

Your group doesn't need to plan much for this, it's a group exercise not a full project. There's no need for a backlog, dev-strategy, wireframe, ... all you need is a a project board with:

• todo - solutions someone wants to try writing
• doing - the solution you are currently working on
• ready for review - solutions waiting for a code review
• done - solutions that have been merged to main/master

You don't need to use milestones, but a label for each behavior (challenge folder) and different strategies can help finding similar solutions for inspiration. There is a template Issue and a template Pull request in this repository that point you in the right direction.

Besides that, just go for it.

---

Code Review

Writing your solution is only half of the exercise, reviewing each other's code is the other half. You've already had some experience with reviewing HTML and CSS in Incremental Development, so you know that it's important to look over each others' work before merging to main/master.

Branches and PRs

Develop every single solution on a separate branch then push each branch to GitHub when it's ready for a review. There should be 1 issue, 1 PR and 1 code review for each and every solution submitted. You'll know you're ready for review when you've

1. Formatted your code
2. Passed your linting checks
3. Passed all your tests
4. Written your README

When you open your PR be sure to use the Pull Request Template for code review. After you've opened your PR it's time to:

1. link your PR to your solution issue on the project board
2. request a review from 1 or two classmates
3. get started on a new solution

The Checklist

No code review is complete without a checklist. This repository has a Pull Request Template for code review with all the things to check in your code review. The checklist covers 3 different aspects of the code:

• Behavior: Is the code well documented? Do the tests make sense? Do the tests pass?
• Strategy: Is the strategy clear and well explained?
• Implementation: Is the code well-formatted? Does it pass the linting checks? Are variables well-named? Is the code clear to read and understand?

This checklist is very long to help you learn what's important when reviewing code. Code review checklists in future projects be shorter and less detailed than this one.

---

Repo Setup

Not much to do, you aren't building a web page so there is no need for boilerplate code or GitHub pages. All that matters is:

• Everyone in your group is a contributor with write access in the group repo
• in the Branches section of your repo's settings make sure:
  • The repository requires a review before pull requests can be merged.
  • check Dismiss stale pull request approvals when new commits are pushed
  • The master/main branch must "Require status checks to pass before merging"
  • The master/main branch must "Require require branches to be up to date before merging"
• In enable the Actions section of your repository so the automated checks will run each time you send a PR to master/main

---

Local Setup

So you're ready to start coding? If you haven't cloned this repository already you should, and then ...

1. Clone this repository:
   • $ git clone [email protected]:HackYourFutureBelgium/practice-code-review.git
2. Navigate to this repository in your local computer
   • $ cd practice-code-review
3. Install the project's development dependencies - you will need these for the code quality automation:
   • $ npm install

If you would prefer to develop your code with the browser's devtools, you can use Study Lenses:

1. $ npm install -g study-lenses
2. Navigate into this folder on your local computer
3. $ study
4. The directory will open in your default browser, and you're good to go.

---

Developing your Solutions

You can write, run and test your code in Node.js or the browser. Whichever you prefer.

• In the Browser: you can use Study Lenses to edit, lint, format, test and debug your code from the browser. Just make sure that the environment open is selected and set to module
• In Node.js: you can also do everything directly from VSCode using the terminal to run your code (node path/to/file.js), and the debugger to step through it. You will need to use the |> current spec file option to debug a file with tests.

---

Code Quality Automation

Writing code is hard. To write even just 10 lines there are 100 things you need to think about, and 1000 mistakes you can make. Developers are clever and lazy people. They have built tools to help with all of this.

NPM Scripts

This repository's package.json has 4 scripts to help write the cleanest code you can:

Formatting

$ npm run format

Formatting doesn't change your code's behavior but it sure makes your code easier to read. It's hard enough to read your own code, now imaging trying to read everyone's different code!

Automated formatting helps everyone in your project write their code with the same style so that reading your own code is the same as reading some else's.

The npm run format script in this repository uses Prettier and a local configuration file to format the code in your .js (and .md) files.

Linting

$ npm run lint:js -- path/to/file.js

Linting is a type of static code analysis - that means it checks your code for mistakes without running it. Kind of like when you read your code to look for mistakes, but automated.

The linting configuration in this repository is strict. There is a lot of code you have read and written in the last weeks that would not be allowed. This is hard to get used to at first. But once you get used to this you will find strict conventions are helpful for writing consistent code, avoiding easy mistakes, and collaborating on a code base.

The npm run lint:js script uses ESLint Node.js API, eslint-plugin-jsdoc-rules, eslint-plugin-spellcheck and a local configuration file to check your .js files for possible mistakes, bad practices, and consistent style.

• npm run lint:ls: checks all the folder & file names in your project to make sure they fit the project conventions.
• npm run lint:md: checks all your markdown files for syntax and style mistakes.

Testing

$ npm run test -- path/to/file.spec.js

You've already practiced with tests in the last weeks, unit tests in this repository will be exactly the same - describe, it and expect(_).toEqual(_). The main difference is that your tests and your function will be in two different files:

• solution-name.js: this file exports your function
• solution-name.spec.js: this file imports your function for testing

The test script uses Jest to run your tests and report the results.

Code Coverage

Code Coverage is a measure of how many lines of code in your function are executed during testing. For example, a unit test for a solution that uses conditional statements does not have complete coverage until every path of the conditional is executed while running the tests.

Each time you run the testing script in this repository it will build a report of your the most recent tests' coverage in ./coverage/lcov-report/index.html. For example, if you test one challenge's tests the report will contain info for only that challenge. If you run all the tests (npm run test -- ./src) then there will be a report for every function in the /src directory.

These reports will not be pushed to GitHub, but having poor code coverage will cause your CI to fail. So it's a good idea to read through the code coverage report for your solution before pushing.

Careful! 100% code coverage does not mean 100% perfect code, it just means you have tested every line of your code. If your test cases are not correct, or if your function does the wrong thing, then perfect test coverage is not helpful.

Here are some links that can help you interpret the reports:

• Amy Pellegrini (stackoverflow)
• Atlassian
• Software Testing Help

Continuous Integration

For now you can think of Continuous Integration is a fancy way to say "automatically check your code before you merge". This repository has 3 CI scripts to help your group maintain a quality and consistent code base. If any of the scripts encounter a mistake you will not be able to merge.

hint: remember to enable GitHub Actions in your repository!

lint.yml

npm run lint:ls && npm run lint:md && npm run lint:js -- ./src locally to check if this action will pass

This action will run a linting check on the ./src folder each time someone sends a PR to or pushes to main/master. All of the linting checks must pass before you are allowed to merge changes.

Don't forget to use npm run format before pushing! The linter will also check your code's formatting.

test.yml

npm run test -- ./src locally to check if this action will pass

This action will run all of the .spec.js tests in the ./src folder each time someone sends a PR to or pushes to main/master. All of the linting tests must pass before you are allowed to merge changes.