Skip to content

Make Florie Guibert a Frontend Maintainer

Florie Guibert requested to merge 12875-fguibert-maintainer into master

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.

I haven't been a trainee for very long but I've been reviewing for much longer. I've also been a gitlab-ui maintainer for a few months which was helpful as a stepping stone toward becoming a maintainer for the main codebase.

Special thanks to @ekigbo for mentoring me these last few weeks and helping with this process.

Examples of challenging situations

I cannot think of any particular awful situation I've been in. When I introduce a bug, I'm never on my own and I always feel supported. If a revert is needed, I have no issue doing it. Sometimes the maintainer who merged the MR introducing the bug revert it themselves. Then I go back to it, create a new MR and ensure the bug is gone in the new version.

Sometimes reverting is not what makes the most sense and I'll decide to quickly submit a new MR fixing the issue, working closely with the maintainer who reviewed and merged the original MR.

So what I've learned, is that we're never single highhandedly responsible for mistakes happening. Maintainers are humans too and we all make mistakes. We're together in making them, and we're also together in fixing them, which is something I truly appreciate here at GitLab. It's probably why I've never panicked when something bad happens. I jump on the incident channel or on a call with a maintainer and we fix the issue.

This MR is an example of a change that introduced a bug. It made it impossible to create a board with an iteration. With it being behind a feature flag (and without letting anyone have time to realise there was a bug), I submitted a fix and sent it straight to the maintainer of the initial MR.

Links to Non-Trival MRs I've Reviewed

MR Diff Key takeaways
PipelinesUsageApp: add project list and their CI usage 627 -36 This had been through an initial round of review so I thought it was interesting that I still had feedback to add to it. It's quite a large change (>600 lines) and touches on different aspects of the code (vue, ruby helpers, graphql). Some tests were missing and I made some suggestions to improve readability. I was mostly confused about the use of Strings for Booleans and had to double check that those variables could never be anything but true/false before suggesting to cast them. This is quite outside my domain knowledge as well.
Update issuable confidentiality UI & status text 222 -41 This MR contained a decent amount of code repetition and before suggesting to create shared components, constants and utility functions, I had to make sure it made sense.
Deprecate manual iteration management for automatic iteration cadences 334 -237 This is quite a large change (>300) that involves a feature flag. A lot needed to be tested to ensure everything was working as expected with the feature flag on and off. It also involved the use of Vue Router which I'm not familiar with.
Update project compliance framework settings to be more clear 297 -38 This is very much out of my domain knowledge and a decent size change (~300). It involves the addition of a Vue component while making a lot of changes to a related HAML template. The mix makes it harder to review than having all the changes in the same language in my opinion.
Remove jest test callbacks from API specs 215 -251
Remove jest test callbacks from IDE specs 763 -1176 These two MRs were hypnotising to review as they are large repetitive changes. It becomes hard to pin point mistakes after 20 minutes of reading what seemingly looks the same.
Generalize epic filtered search component for board reuse 314 -221 There was a decent amount of code to tidy up there, while also ensuring existing uses of the component were still working as expected.

Trainee issue for more: #12875 (closed)

To summarise, here are the things I look for and feedback I tend to leave:

  • I ensure the description is clear enough, with screenshots/videos, steps to reproduce and potential mention of a feature flag. It's happened many times that a feature flag was involved without its name clearly written in the description, making things harder to test.
  • I ensure that required counterparts are involved when needed (notably QA and UX)
  • I manually test the change locally. The most important things to me is that it works, the code looks reasonable and there's no performance impediments.
  • I tend to leave comments about migrating bootstrap CSS utility classes to gitlab-ui classes and avoid adding too much new CSS.
  • I ensure tests coverage is good and that they make sense. I always have this epic handy when making suggestions on how to improve tests so we don't keep using old standards.

Links to Non-Trivial MRs I've Written

Note: Building the frontend for Swimlanes while being the feature's DRI is probably the least trivial thing I've done but there are too many MRs to list and it's within Plan domain knowledge so here is the related epic. The MVC work itself spanned over 9 milestones.

Note 2: Most of the features I work on as part of the Plan stage are quite complex, so this is just a selection to demonstrate various aspects I've been working on.

Label component migration

MR Diff Comments
Replace labels in Vue with gitlab-ui component 158 -368 This was a huge undertaking, which I started very early after joining GitLab. Labels are everywhere and it was the very first gitlab-ui component to be integrated into the main codebase. I learned a lot in a short amount of time about our codebase and the technologies we use.
Update labels on sidebar update with gitlab-ui css classes 55 -26
Replace labels in Haml with gitlab-ui CSS 285 -158
Docs on integrating components in GitLab 19 -0 Since this was the first gitlab-ui component to be migrated in GitLab, I wrote some documentation about the process to help engineers with migrating other components.

Sidebar widgets

MR Diff Comments
Sidebar notifications subscriptions widget 536 -198 All these MRs are similar in terms of architecture while varying in terms of effort. I participated in the effort to refactor and migrate sidebar components to be standalone reusable components. By owning the queries and mutations, they increase performances of pages with sidebars. They are also versatile components that can be used for Issues, Epics and Merge Requests. This was an amazing learning experience, with GraphQL and Apollo mostly.
Due date issue sidebar widget 392 -42 ------
Add To Do sidebar widget 466 -31 ------
Milestone sidebar widget 1075 -344 ------
Milestone widget for Issue and MR page sidebars 165 -72 ------

Labels widget

MR Diff Comments
Tidy up labels widget 35 -1057 Labels was the most complex widget and I wasn't the only engineer involved. I took over the development of this component after Natalia did the heavy lifting. This MR cleans up legacy code from previous labels selector.
Labels widget architecture polish 48 -70 Some refactoring so that the widget follows the other widgets' architecture
Clean up labels_widget feature flag 140 -658 Removes the feature flag once the widget was usable.
Add Labels widget support for epic 227 -87 Extends the widget to make it possible to use it to assign labels on epics.
Use labels widget on epic board and MR sidebar 222 -57 Migrates selector to widget in more places.
Labels widget - Fix search input 311 -133 UX polish to improve usability
Board sidebar - Migrate labels select to widget 177 -62 Migrates selector to widget in more places.
Consolidate labels widget architecture 282 -119 This was the most complex widget to build and migrate and it was definitely a team effort. It required a lot of iterations and polish but having it behind a feature flag for a while really helped with this process. It was easier to find and fix bugs once it was in front of users though. A few steps like this one were just some non user facing refactoring to align its architecture with our code guidelines and other widgets. When we started this work, we just copied the existing selector as it was already a complex composition of Vue components. But it was old and needed some serious refresh which could only be done over multiple iterations

Documentation

MR Diff Comments
Work items widget frontend architecture 114 -0 I used what I learned when building and migrating sidebar widgets to make some architecture recommendations about how we are going to build widgets for work items
Epic boards documentation 68 -0 Just feature docs
Epic boards docs update for 14.0 release 195 -58 Just feature docs
Update docs for Milestones in Roadmap 27 -20 Just feature docs
Roadmap settings documentation 18 -0 Just feature docs

Introduce Cypress e2e test in gitlab-ui

MR Diff Comments
RFC I've taken an interest in gitlab-ui pretty early on in my time at GitLab. As a contributor it became obvious that it was necessary to add the ability to test complex components before they get integrated into our main codebase. It was not hard to convince the frontend team that testing compositions of components would make our library more robust and would prevent some issues when integrating them into GitLab.
MR 547 -26 This MR introduces Cypress, documents the process of adding tests and adds a pipeline job to run it. It adds tests for one component as an example. The Frontend team happily adopted Cypress and our end to end test coverage in gitlab-ui is progressively increasing.

Some roadmap contributions

MR Diff Comments
Add hierarchy depth to roadmaps 1045 -262 This was my first contribution to Roadmap, which is quite a complex feature. Learned a lot about it, as well as epics.
Add milestones to roadmap 1467 -82 This was before I knew about feature flags. I was still of the school of avoiding submitting MRs without adding user value. After this gigantic MR, I learned to iterate while being behind a feature flag.
Update docs for Milestones in Roadmap 27 -20 Feature docs update for the above

Other

MR Diff Comments
Rspec wait_for_requests helper waits for graphQL requests 42 -11 I made this change following recommendations of a backend engineer but it was super interesting to make changes to rspec helpers. This was critical for me to be able to write end to end tests for the features I work on which often use GraphQL.

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 Florie Guibert

Merge request reports