Skip to content

Add Alexander Turinske as frontend maintainer of GitLab

Alexander Turinske requested to merge aturinske-master-patch-64097 into master

Self-assessment

After >2 years at GitLab and over 10 months as a trainee maintainer I want to propose myself as ~Frontend maintainer. In this merge request description I have listed for you my justifications and the relevant and merge requests that I have reviewed and authored. Additional reviews can be found in my trainee maintainer issue.

There are many reasons I want to be a ~Frontend maintainer:

  1. I like mentoring; I have been an MR coach for ~1yr and have found it rewarding to promote good coding practices and to keep GitLab moving forward with new MRs
  2. I think reviewing code is one of the best ways to learn more about software engineer and the company by seeing many different coding styles and features
  3. I want to help lessen the current load placed on current maintainers by sharing in the work

There are many lessons I have learned:

  1. To be more cautious; I have approved code that "looked fine", which ended up being caught by a maintainer for causing a bug. This has taught me to test the code I approve (when possible).
  2. To provide thorough reviews consistently; in the beginning of the process I found my reviews to swing from thorough to reckless based on my mood. This lead to an inconsistent review experience from me where some MRs got through without any maintainer feedback to the scenario listed in #1. With this self-reflection, I have learned to better recognize the mood I am in and take steps toward being in the reviewer mindset.
  3. To bring in a domain expert for a second-opinion if I feel out uncomfortable about reviewing something I don't know much about; I don't know everything and it never hurts to get a second reviewer.
  4. To use feature flags on high-visible features; from working on the Secure/Protect group where Ultimate customers are paying for some features I touch, it feels bad to introduce a breaking change. It is much easier to just create a feature flag.
  5. To be proactive with complicated MRs; if I think the MR is too complicated, it should be broken up before someone asks me to. If I think it cannot be broken up any further, I add comments explaining the complicated portions of the MR to give the reviewer context.
  6. To ask Why is this like this? in an MR; not all MRs come with the full context they need and it is better to ask Why? than to assume that a weird change is somehow justified by a reason you don't know.

Requirements:

  • Advanced understanding of the GitLab project as per the documentation:
    • I have authored and reviewed merge requests that cover multiple technologies used in the GitLab codebase, this includes Vue, Vuex, GraphQL/Apollo, HAML, Jest, SCSS, HTML, internationalizations best practices. Additionally I have authored and reviewed a wide range of changes that touch multiple areas and tools in the GitLab codebase, such as agents, projects, groups, and admin-level tools. I have also taken part an active role in several Pajamas migrations (such as alerts and buttons that have taken me all over the codebase.
  • The MRs reviewed by the candidate consistently make it through maintainer review without significant additionally required changes.
    • In addition, I am mindful creating feedback that is blocking and I am in favor of creating follow-ups when a suggested change is outside the scope of the original change to keep up the author's momentum.
  • The MRs authored by the candidate consistently make it through reviewer and maintainer review without significant required changes.
    • In addition, I am open to any recommendations and questions, and when I am unsure of or disagree with a recommendation I try to communicate that openly, look for precedent and ask for additional feedback from another reviewer when possible.

Links to Non-Trival MRs I've Reviewed

MR Changes Notes
Add the pipeline wizard step component (gitlab-org/gitlab!81412 - merged) +424-0 This MR had good code, but the tests needed an overhaul; they were more integration tests, testing how several components worked together rather than ensuring the one component was working properly, and I coached the engineer in good testing practices.
Add scan result policy into the UI in both policy (gitlab-org/gitlab!75689 - closed) +660-45 This MR had good code, but was trying to do too much at once; I suggested it be broken up into several small MRs and the suggestion was accepted! 🎉
Apply new GitLab UI to search and dropdown in h... (gitlab-org/gitlab!52087 - closed) +40-34 This MR looked trivial, but I ended up having to dig into our implementations of dropdowns in HAML and verify that this feature didn't "just work". That resulted in me having to tell a community contributor that we would not merge their work and that felt bad, but that is how it goes sometimes.
Step 3 - Use ide.files.change for live preview (gitlab-org/gitlab!50255 - merged) +125-41 This MR was in a complicated part of the codebase and was part of a complicated project.
When no iterations are present show empty state (gitlab-org/gitlab!79067 - merged) +128-44 This MR had a lot of good comments by me.

I can catch bugs!

1, 2, 3, 4, 5, 6, 7, 8, 9, 10

I can review code and not have any significant changes added by maintainers!

1, 2, 3, 4, 5, 6, 7, 8

I enforce best practices (i.e. the code worked, but I suggested an improvement)

1, 2, 3, 4, 5, 6, 7, 8, 9, 10

I know can defer changes to follow-ups

1 (there are more, I just cannot find them 🤣 )

Links to Non-Trivial MRs I've Written

All my contributions at GitLab can be found here: https://gitlab.com/dashboard/merge_requests?scope=all&state=all&author_username=aturinske

MR Changes Notes
Add cluster filter to operational vuln report (gitlab-org/gitlab!82764 - merged) +211-13 This MR had complexity because of the domain, all the edge-cases, and a seemingly endless review; it was iterated on several times before its final, simplier form. It also got merged with a bug, which prompted the creation of a feature flag to hide it, which I should have done from the start. TIL
Remove network policies feature from the UI (gitlab-org/gitlab!82319 - merged) / Remove threat monitoring sidebar option and routes (gitlab-org/gitlab!82315 - merged) >+4000-0 I deprecated part of a features that were intimately entwined the rest of the feature and luckily I had isolated some of the code, but also there is only so much I could do. The two attached MRs were the first MRs that removed the usage of the files I planned to delete, which made it so I could iteratively remove the underlying code without concern of making sure the feature still worked (it also ensured the feature would get deleted in 15.0. I originally took a non-iterative approach to this, but course-corrected upon that realization (and the realization that the MR created was going to be a nightmare to review 🙃 ). For the full list of MRs involved in deprecating the features, see Remove Network Policies (gitlab-org/gitlab#352285 - closed) and Remove Threat Monitoring page (gitlab-org/gitlab#352287 - closed).
Rename threat_monitoring to security_orchestration (gitlab-org/gitlab!88386 - merged) +157-152 I renamed an entire directory in GitLab and had to continue rebasing the branch while people were still working on it and that lead to files being left at their old paths. It seemed simple at the beginning, but I very much wanted to implement a directory lock by the end.
Update policy list root file to be malleable (gitlab-org/gitlab!83911 - merged) / Add group level policy new page (gitlab-org/gitlab!84393 - merged) / Add group-level scan execution policy retrieval (gitlab-org/gitlab!87081 - merged) >+300<-100 I generalized a project-level to work also at the group-level. Walking through the code and figuring out what needed to be generalized was non-trivial, but it was nice to see that I had created the feature in such a way where it was not too hard to generalize. The whole project was interesting and consisted of many MRs, but these three are the most interesting
Allow Users to Edit yaml-mode Scan Execution Po... (gitlab-org&5362 - closed) >+1000-0 A big learning opportunity for me; I moved a page AND added a new feature to it at the same time and that was an anti-iteration mistake; doing both at once opened up many opportunities for errors and dragged the project out for >3 milestones. I should have done one of them behind a feature flag, waited to ensure there were no problems, and then removed that portion from the feature flag and started on the other

Examples of challenging situations

See the MRs I have listed above; in there are several examples of situations I found challenging and the lessons I learned from them. Additionally, the list of lessons I have learned in the Summary sections are all examples of what came out of challenging situations.

Overall, I have found the toughest part for me to has been to learn to say "no, I will not approve this" for code that I do no think is good enough. It feels bad, but I found that it doesn't need to; instead those situations are perfect teaching moments and should be used to their fullest potential to help everyone improve.

Being an MR Coach

I have found that being an MR Coach has been rewarding because I get to mentor often on how to effectively contribute to GitLab, but it has also been challenging because people want to contribute but they don't always know the best practices we use. When I first started mentoring I would often clean up the MRs for contributors (1, 2) so that they could get their MRs merged faster/easier, but quickly found that to be unsustainable in terms of the time commitment. Eventually I learned that it is better to train contributors to conform to our standards so that their MRs can be merged faster/easier (1, 2, 3, 4, 5). I believe that was advice given to me by @pslaughter and I have found it to be incredibly useful. 🙇 :thank_you_emoji_that_still_is_not_in_GitLab:

Next Steps

@gitlab-org/maintainers/frontend please chime in below with your thoughts, andapprove this MR if you agree.

Once This MR is Merged

  1. Create an access request for maintainer access to gitlab-org/gitlab.
  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 Alexander Turinske

Merge request reports