Doing code reviews is a skill
Tags: testing, opensource, webdev, javascript
Published: 2022-11-19 06:43:16

A wise man once said doing code reviews is _a skill like writing code_

I agree. 

Like in the beginning, you need training wheels to get comfortable riding a bicycle. Then, eventually, you do not need them anymore. It's the same with _coding_, and _code reviews_. To get better at something, you need to practice it frequently. 

In the last few days, I got to do code reviews (for the first time in my life) in [My Photohub](https://github.com/SerpentBytes/my-photohub) - A fantastic project being put together by a great team of developers. 

My Photohub is a web app that makes it easier to share your photos on the web. You can read more about it [here](https://github.com/humphd/my-photohub). 

I also did a code review in [vs-code-seneca-college](https://github.com/Seneca-CDOT/vscode-seneca-college) extension in development, which aims to make collaboration for developers at Seneca College easier.

## Review #1

{% github https://github.com/humphd/my-photohub/pull/10#issuecomment-1312852282 %}

### My Reasoning
GitHub PAT tokens are like passwords, so they should be masked. However, since My PhotoHub is in its infancy, it helps make debugging easier if, as developers, we can see what we are entering as we type. Of course, we would mask the input before shipping the product. 

### Method of Testing
- [Checking out the PR locally, using `git`](https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/checking-out-pull-requests-locally)
- Using [depscan](https://www.npmjs.com/package/depscan) - a tool that scans for unused dependencies in a project

View the PR [here](https://github.com/humphd/my-photohub/pull/17)

## Review #2
![screenshot of code review #2](https://dev-to-uploads.s3.amazonaws.com/uploads/articles/hzpuz2nxz9uy2dzjpb9g.png)

### My Reasoning
If we add more `workers` down the road, we need to modify the `CONTRIBUTING.md` file to add a `publish` script for each `worker` we introduce. By making the `script` generic, we are crossing out one extra `commit` per new `worker`.

View the PR [here](https://github.com/humphd/my-photohub/pull/17)

## Review #3

![Screenshot of code review #3](https://dev-to-uploads.s3.amazonaws.com/uploads/articles/309z9q487673znyubbef.png)

### My Reasoning
The content must be free of grammar errors, for sure. However, you also need to ensure the document meets accessibility guidelines. According to an [article](https://usability.yale.edu/web-accessibility/articles/headings#development) published on the Yale University website, _when using a sectioning element, a heading should generally be the first content within the section to act as a label._ 

Content preserves the _reputation_, but presentation preserves the _audience count_. Both are equally important.

### Method of Testing
[markdown-viewer](https://codebeautify.org/markdown-viewer) - it beautifies markdown, but it also shows the output

View the PR [here](https://github.com/Seneca-CDOT/vscode-seneca-college)

## Learning Outcomes
I learned that I need to understand the code to review it. 
Also, I need to weigh the pros and cons of the changes that the pull request will introduce in the codebase upon merge.
Finally, I should checkout the pull request for projects that have a UI locally and be able to test it.