Skip to content

Add Anna Vovchenko as a frontend maintainer

Summary

Total MRs I reviewed: 120
MRs were merged as is: 87 (77%)
MRs were merged after implementing small suggestions from the maintainer: 24 (21%)
MRs needed a larger input from the maintainer: 2 (2%)

My goal as a reviewer is helping to successfully get an MR through its way in the review process to the production. It is important for me to be sure that the code is clear, well-documented, and covered with tests. I aim to ensure that the changes are consistent with the rest of the codebase and our style guides. I often bring links to the development guidelines and try to always illustrate my points with the actual code. I test every MR manually and run unit tests locally. I feel comfortable asking for another opinion and requesting an additional review if needed. I am trying to be as much efficient as I can and reduce the time an MR spends in the review process by differentiating my suggestions as blocking or non-blocking, preapproving if nothing is blocking, and requesting the maintainer review when ready.

My trainee issue: #12443 (closed)

Links to Non-Trival MRs I've Reviewed

MR Changes Weight (out of 10) Notes
Introduce new Colour Select Widget (gitlab-org/gitlab!78889 - merged) +1055 -0 8 It was a large complex review. I communicated a lot with the author - both sync and async. We had a pairing session to develop the needed specs. I tested everything manually, helped to spot some potential issues and UX problems. This MR is a part of a bigger afford the author was working on for almost a year.
Add cart abandonment modal (gitlab-org/gitlab!79033 - merged) +456 -20 4 As a part of my review, I was trying to maintain best practices from our style guides. I followed our review guide and suggested requesting the product intelligence review for the tracking events. I tested the events locally and spotted a potential bug leading to the event being fired twice.
Allow linking CRM organizations to contacts (gitlab-org/gitlab!86255 - merged) +290 -106 4 I shared my ideas on the UX and helped to make code more clear and clean. I checked specs thoroughly and spotted the way to increase test coverage together with deduplicating specs and included rewritten code for the author's consideration.
Refactors project packages & registry settings ... (gitlab-org/gitlab!89377 - merged) +320 -284 3 I spotted some dead code, enforced Pajamas guidelines and caught a UX bug.
Clipboard button helper classes (gitlab-org/gitlab!84783 - merged) +8 -20 2 The changes were straight-forward, but I've found similar issues across the codebase and suggested a patch to update it. I was keeping an eye on this MR while it was stale for 2 weeks and acted proactively fixing the failing spec, requesting the maintainer review, and getting it merged.
Fix: notify locale on removal notification (gitlab-org/gitlab!90324 - merged) +11 -9 1 This is an example of a small change, which I still tested manually and was able to spot a not-so-obvious bug.

Links to Non-Trivial MRs I've Written

MR Changes Weight (out of 10) Notes
Add view all tab to the clusters page (gitlab-org/gitlab!74135 - merged) +741 -34 10 It was a part of series of 6 MRs with total changes of +1399 -280. It included major refactoring, migrating old haml components to vue, developing new navigation, implementing new views using Pajamas compliant components, and updating a lot of tests on multiple levels. First started as a single MR it helped me to learn the best ways of code splitting and iteration.
Agent Activity Information - Frontend (gitlab-org/gitlab!76118 - merged) +742 -8 6 This MR is an example of the independent frontend delivery while waiting for the backend implementation. We communicated clearly with the team on the data structure, and I was able to prepare multiple event types before they could appear on the backend. We were able to smoothly deliver new events on the backend without having to change the frontend code.
Refactor Terraform Widget to extension (gitlab-org/gitlab!76780 - merged) +430 -6 5 Within this MR I introduced the new terraform merge request widget, which appeared to be the first MR extension to use polling. Its polling method was then refactored to serve the other widgets (not by me, but I took part in reviewing) and helped to unblock developers waiting for this feature.
Add clusters Actions menu to group and admin views (gitlab-org/gitlab!81846 - merged) +250 -178 3 Working on this MR was mostly a refactoring that helped reduce duplicative code and develop a single approach for the clusters page on all levels - project, group, and admin. It helped to maintain the code easier and reduced the work related to clusters deprecation.

Examples of challenging situations

The one when I introduced a bug and noticed it

Add clusters Actions menu to group and admin views (gitlab-org/gitlab!81846 - merged) is one of the MRs I am proud of. But I did miss updating one haml file that led to having duplicated button on a group level view. I spotted the bug as soon as the MR was deployed, and prepared a fix. It turned out that the other team member was also working on a fix, but there was enough room for both of our improvements. I've learned that it's important to communicate about a bug as soon as possible to avoid having two people working on the same fix.

The one when I introduced a bug and didn't notice it

The customer faced an issue using the agent when they couldn't create a new token from the empty state, but it worked fine for them to create a second token. I communicated in an open issue that it looked like a frontend bug, and I was going to investigate it shortly. I created Fix agent token modal (gitlab-org/gitlab!90644 - merged) later that day, and it was deployed to production the next day and picked for the previous release (as it was already the end of the milestone). Turned out the issue appeared due to having the modal with the instructions twice on the page - one for the empty state and another for the existing token. I think that I learned my lesson from the previous situation and was able to clearly communicate the issue, prevent double work within the team and deliver the fix fast. I then discussed the situation at the team meeting and learned that the MR can be picked into the previous release.

Self-assessment

It's hard to specify hard requirements for becoming a maintainer, which is why the documentation consists of flexible guidelines. Nevertheless, it's still important to have a more formal list to help both a candidate and their manager to assess the "readiness" of the candidate.

  • Advanced understanding of the GitLab project as per the documentation:

    good feel for the project codebase, expertise in one or more domains, and deep understanding of our coding standards

  • The MRs reviewed by the candidate consistently make it through maintainer review without significant additionally required changes.
  • The MRs authored by the candidate consistently make it through reviewer and maintainer review without significant required changes.

Once This MR is Merged

  1. Create an access request for maintainer access to gitlab-org/<project>.
  2. Join the [at]frontend-maintainers slack group
  3. Let a maintainer add you to gitlab-org/maintainers/frontend
  4. Announce it everywhere
  5. Keep reviewing, start merging 🤘 😎 🤘
Edited by Anna Vovchenko

Merge request reports