For release 0.3 of my course in Open-Source Development, I was tasked with reviewing two PRs to any of the Seneca telescope or IPC144 repositories.
For the first review, I looked at a PR to improve the style of the help section for search. The font for the help section did not match the rest of the website. The PR involves changing the font style to Spartan to match the rest of the website and decreasing the font-size as the Spartan font displays larger than the previous font. I tested this by navigated to the vercel deployment link from the PR and confirmed that it worked on different browsers, and that the font looked consistent across different screens sizes. After confirming that everything looks good, I approved the PR. However, someone had pointed out that using [] instead of ' could be better:
This led to a discussion that maybe the search help needed improvements so it could be understood better. Terms such as slop amount, fuzziness were not explained and would not be obvious to someone not familiar with search APIs such as elasticsearch. It was decided that this should be handled in a different issue, and the font change could be merged on its own.
For the second review, I looked at a PR to change the width of the blog URL on the sign-up confirmation page to base it on the width of the parent element. This was previously fixed at 300 px as an approximation to prevent the text from overflowing the parent form element. This was a small change, but it took me a while to test as I had to fix my local setup. To get to the sign-up confirmation page, a test sso is used locally. However, I could not get docker to work. It was stuck in a 'Starting docker' state. Trying to stop it would result in it getting stuck in a 'Stopping' state. Eventually, I solved it by force quitting it using task manager and restarting my computer. Then, I confirmed that the change was working as expected and approved the PR.
Top comments (0)