Skip to content

Add Diana Zubova as a frontend maintainer to CustomersDot

Diana Zubova requested to merge dzubova-master-patch-86046 into master

Self-assessment

I would like to join the maintainers group of the CustomersDot project. My goal is to help keeping frontend code consistent with the main GitLab project, well tested, documented and future proof.

  • It's important for me that the code is clean and fulfilling our frontend guidelines. To achieve it I often link to our guidelines to spread our practices and maintain high level of cohesiveness.
  • I feel comfortable asking another reviewers and maintainers for opinion and do this to get a second pair of eyes when needed.
  • I start the review from reading the description and the actual issue, to understand the why and only then start checking the code.
  • My goal is to be as clear as possible when I communicate to the MR author, the review is my priority work, but when I am at capacity I write back to set realistic expectations or suggest to reassign the reviewer.
  • I always run the MRs locally for manual check and run unit tests.

My trainee issue: gitlab-org/customers-gitlab-com#3984 (closed)

Total MRs I reviewed (GitLab + CustomersDot on 8th of Sep): 80
MRs were merged as is: 48 (60%)
MRs were merged after implementing small suggestions from the maintainer 30 (37,5%)
MRs needed a larger input from the maintainer 2 (2,5%)

Links to Non-Trival MRs I've Reviewed

MR Changes Weight (out of 10) Notes
Update purchase confirmation card to handle static zuora events: https://gitlab.com/gitlab-org/customers-gitlab-com/-/merge_requests/4680 +463-174 8 It was a large MR changing similarly looking error handling. Everything was well covered with tests, however these tests were test events that were never actually emitted by the components touched in the MR. Only checking locally the edge cases reveled this part. Together with the MR author we discussed the problem in the comments and later on a call to develop the approach to error handling.
Validate customer registration form (company and address fields): https://gitlab.com/gitlab-org/customers-gitlab-com/-/merge_requests/4887 +229-140 4 This MR was introducing the emoji validation for two fields. I've spotted that the issue description didn't emphasis adding it to one last missing field so the whole form is covered + proposed a test that would check that
Allow user to edit timeline event: gitlab-org/gitlab!92111 (closed) (GitLab) +872-235 8 I suggested to break the MR into two MRs: one that would refactor the component the way that addition of the extra action would be simpler and one that would handle this addition. I was also reviewing both follow-up events and together with the author we achieved the desired outcome.
Delete useless bold when user select mask variable in ci/cd variables modale: gitlab-org/gitlab!93836 (closed) (GitLab) +1-7 2 Community contribution, small code change but noticeable UX impact. I've checked the issue description carefully and decided to clarify the requirements with UX. I've marked the MR as blocked and opened a discussion in the issue. My feeling that the requirement was not final turned out to be correct. That helped us to not merge something we didn't fully agree on

Links to Non-Trivial MRs I've Written

MR Changes Weight (out of 10) Notes
Split SubscriptionButtons component: https://gitlab.com/gitlab-org/customers-gitlab-com/-/merge_requests/4788 +748-830 6 This is final MR in the series of three that finishes the split of the SubscriptionButtons. While working on another issue I figured out that adding changes and testing SubscriptionButtons became complex and proposed a refactoring that would speed us up later
Add autorenew mailer for self managed subscriptions: https://gitlab.com/gitlab-org/customers-gitlab-com/-/merge_requests/5149/ +139-27 5 In this MR a was picking up already started work, it was important to keep the main idea but improve on test coverage, carefully check the updates manually. This MR has a serious BE part in it and I find it important that in fulfillment we deal with both FE and BE with high proficiency. We had a follow-up to polish the mailers after the product relevant part is merged. I would prioritize product changes first but make sure that the follow-ups are not forgotten.
Use TimezoneDropdown from shared components: gitlab-org/gitlab!96567 (merged) (GitLab) +120-56 4 The main goal of the MR was to migrate the component but while working on it I figured out that we would benefit from changing the API of this component, updating it's structure and in the follow-up cleaning the folder structure. It's an example of the MR when you don't stop when "it simply does the job" but think about future use cases
Update tracking documentation: https://gitlab.com/gitlab-org/customers-gitlab-com/-/merge_requests/5195/ +6-1 1 I was setting the foundation and implementing most of the recent tracking on CustomersPortal and it was important to me that this knowledge is shared with the whole team so I decided to open an extra docs section for it.
Migrate trial banner to AlertComponent: gitlab-org/gitlab!89804 (merged) (GitLab) +9-69 4 This MR started as a mere migration that was supposed to be a one line change but by this hint from the reviewer I turned it into a full exchange from banners to alert and helped later to update and merge a similar MR

Examples of challenging situations

  • One MR got stuck in the review loop for a couple of weeks. I followed the handbook advise to find time and discuss the issue on a sync meeting with reviewer.
  • It also helped me once to bring the MR to the Pair Programming (now Code Review) sync session, when a few more other people brainstorm a complex issue with you it could save days of work.

Once This MR is Merged

  1. Ask a maintainer to add you as an Owner to the relevant maintainers list
  2. Join the [at]frontend-maintainers slack group
  3. Keep reviewing, start merging 🤘 😎 🤘
Edited by Diana Zubova

Merge request reports