Fixing a bug!
I have chosen an issue that looked innocent and important at the same time. The most trouble I had with it was.... RUNNING THE DAMN PROJECT!
I followed an environment setup guide which was pretty damn long. At some point I got stuck, it wanted me to install something that was too advanced to my Windows version. I would need to reinstall my Windows to really fix that, and that would take at least a weak and a shit ton of irritation (Because my windows has no updates.. which never backfired for 10 years until this moment. I'd say still a good trade off.)
In any case, I was offered a solution:
Great! I had a magical browser environment that I could use to do fix my chosen issue.
One problem tho.
You needed to run it localhost to reproduce the bug.
-_-
...
It took me a while to figure out that I only needed to do the npm installs to run only the part my issue related to.
It then took me a while to understand why the issue even occured. I didn't fully understand the document structure they had. It still is a mystery to me how they got
service.router.use(process.env.PATH_PREFIX || '/', static(path.join(__dirname, '../public')));
PATH_PREFIX
From a totally different place - env.local
They didn't import anything, they didn't read the file... Idk man. Oh well. Magic it is. It works - it works. I don't touch.
What caused issues basically was the undefined variable. In env.local you had some local variables, where you specified where you should be redirected by default. However, when the page would not read that variable, or did not have it specified, it gave 404.
This part would redirect to the specified variable in env.local:
if (process.env.PATH_PREFIX !== '/') {
service.router.get('/', (req, res) => {
res.redirect(process.env.PATH_PREFIX);
});
}
Here it decides which path to use. If PATH_PREFIX is undefined, it goes for '/'.
service.router.use(process.env.PATH_PREFIX || '/', static(path.join(__dirname, '../public')));
This small part does a similar thing. If we didn't specify API_HOST, it goes for 'localhost'.
const host = process.env.API_HOST || 'localhost';
This way when we do get undefined variables, our webpages don't crash, but go for a different option. My PR didn't seem big, however I needed to understand how the whole thing worked before I could add those lines.
Code reviews
A small versioning upgrade
This one was a fresh PR that was really simple. A version of traefik was upgraded to a newer one. I just needed to see if stuff still works when it was. And, well... it did still work. No fancy magic here.
Formatting
My second review was also on a fresh PR. It was a simple formatting upgrade, which I checked. It was okay. However, I noticed something. In the requirements it was mentioned that people should improve images used to look good in dark versions. I checked and saw an image that would have its text not readable on a black background. I thought - I can fix that, I know photoshop! So I did. And I sent my result in the PR so that its author can add it to their changes. Hopefully I did it right. Well, if not - they can just decide to not use it.
I generally didn't enjoy reviewing code as much. But I suppose someone has to do it.
Top comments (0)