I know I'm supposed to work on the microservices, but these frontend code reviews are killing me... >.>
So Version 1.9.1 of Telescope is now released. 1.9 had a hiccup so we had to make a hotfix release. As long as the work is done, that's what matters.
I spent most of the week reviewing front-end code. There was alot of PRs in for 2.0 design fixes so I needed to look at those. Front-end code has always been something I enjoyed looking at and working with, and although I don't get much of a chance since many in our group prefer working on the front-end, reviewing them is always a breath of fresh air for myself. I might even pick up a smaller front-end issue in next week's meeting.
Anyway, here's a general list of PRs that I reviewed this week:
- Improve user accessibility of linking to AAA
- Change color for search bar in dark mode
- Don't center table contents in figure
- Correct avatar link in postInfo and rename avatar prop
- Shrink the title font-size; add author blog link to avatar
- Fixes #2000 redirect URL error
- Fixes issue-1929: Paginated get route
- Create temporary env-setup for microservices transition
Probably my biggest list of reviewed PRs yet. As many of my issues require me to look into parts of Telescope I haven't looked at before, I used this time to look into PRs that others made so that we can get them in. Sometimes, I mess up and others have to correct me, but that's why we always have 2 reviewers required to approve in order to merge (Looking at you, 2022).
In terms of work I did, I made a PR to switched the SearchResults component to use the microservice urls instead of the old telescope backend url. This was all fine and dandy until....
So uhh, the SearchResults component makes a query to elastic-search. This will return the query and other parameters that the component needs in order to display it's results. I made the mistake of using the posts microservice url, which doesn't have a route that returns though queries. As a result, nothing shows up. We have another PR that adds the search microservice up, but the owner, Ray, has been slow with updating it so I was tasked with continuing his work, which is now my priority. Once that's merged in, we can add it to production, and switched the frontend to using that url instead of the posts service url. You can find the PR here.
I think the main thing this week is realizing how much more work I'll need to do in order to get better with performing code reviews. Looking at this PR for example, I blocked the PR from progress because there wasn't enough adequate testing of the PR to prove that it can work in all situations. Another block from David was more because the way it was being done was not the best solution in general. A different reason for this but one that probably rings more true than my reasoning because of the evidence to back up his point. This piece in particular made me think alot,
"However, this breaks how hyphenation works on the web. Hyphens are supposed to provide wrap opportunities to the browser, and removing that seems wrong.
I would expand the width of this container. We have more room on the page, why limit it to such a narrow region?"
Is the code working good enough? Or should we really question the way the code is written? There are good solutions and there are bad solutions. Both will work for software, but one is easier to maintain and makes more sense for developers while another solution does neither. It's not something I consider enough when reviewing front-end PRs and so it's something I need to really work on more.
In any case, that about sums up this blog post. Only 2 more releases left until the 2.0 release. Big stuff are going to be coming. We got 4 weeks left to get all this stuff in on time. Let's get to work! Until next time, stay tuned!
Top comments (0)