Add Savas Vedova as a FE maintainer to GitLab project

Introduction

Trainee Issue: #8059 (closed)

Welcome to Savas Vedova's maintainer issue 👋

Hi everyone! I've had the chance to work closely with some of you already, but for those who we didn't had the chance so far, I'm Savas Vedova. I joined GitLab back in March 2020 and currently working in groupthreat insights. I've decided to try my chance in becoming a ~Frontend Maintainer as I started to feel confident with the codebase and also to help out other maintainers (I know we're overloaded 😅).

Self-assessment

• Advanced understanding of the GitLab project as per [the documentation]

Here's my case 🤶🏻

I usually consider myself as a fast-learner. I try to learn from my mistakes, therefore I'm not afraid of making them. For example, during the second week of my employment I created an MR to fix a priority:1/severity:1 bug. It got merged quickly and we prevented more users to be affected from this bug. Over the time, I created 104 MRs in 8 months of which only 2 of them were closed and the others were mostly merged without a big discussion.

List of Non-Trivial MRs Reviewed

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

So far I was able to review more than 70 MRs. Most of them are listed in the trainee issue but here I'm going to list some of them in order to provide more context in this MR.

This was a great large scale MR series. Took a bit time to review them but at the end we managed to merge them without a huge effort!

✅ Implement vulnerability counts in basic security MR widget ✅ Implement CE security_reports Vuex store ✅ Extract and move Secret Detection Vuex module to CE ✅ Move SAST Vuex module to CE [RUN AS-IF-FOSS] - This is actually a post for an MR series. They all belong to the same author. In order to limit the context switching, the author thought of assigning these MRs to the same reviewer and maintainer. It made a lot of sense since the reviewers gather more context as they go through the MRs and the review process becomes easier. - Given the length of the MRs I followed different strategies to go through the code and find the best way to review them. I left comments on how I reviewed the MRs to share my experience and help the maintainer to review them faster. For instance here, I shared a mistake I did while going through the MR commit-by-commit and suggested to the maintainer that I found it easier to review it as a whole. Here, going through the MR commit-by-commit made my life much easier, so guided the maintainer accordingly. - Suggested a code change to increase performance which got implemented. - Had a small talk with the author regarding splitting the MRs to smaller chunks. At the end, got a great feedback from @oregand where he shares his experience on reviewing large scale MRs

Here I caught a bug before going to production 🚀
✅ Add Vulnerability Severities count to instance report This was a great MR which added vulnerability severity counts to the group/instance level. The code looked great but I managed to Identify a bug while testing the changes.

Here I Identified a few issues
✅ Setup Vuex state needed for group members filter bar * The context was not clear to me so I asked guidance on it and if possible more screenshots to understand the before/after changes. * Identified optional properties that were not needed * Suggested to use parseBoolean helper

A few community contributions here	Context
gitlab-org/gitlab!47626 (merged)	This was a fairly simple change but the author was struggling on how to visualise the component in the UI. I went further and explained our HAML structure.
gitlab-org/gitlab-ui#879 (closed)	This issue was idle for a while. I jumped in, reanimated the conversation and helped the issue to be closed.
gitlab-org/gitlab!41019 (merged)	Guided a community contributor here and did the initial review.

🏆 And the winner is... 🏆

🛑 Network policy editor model This was a special MR. I always wondered what happens when the author and reviewers do not agree on a given subject and I had the chance to learn it in this MR. * I left a comment regarding my concerns on storing the whole class instance into component state: gitlab-org/gitlab!37660 (comment 386779585). Since my concern was rejected by the author, I left the discussion open for the maintainer. After the maintainer was on the same page and he also got rejected, the maintainer weighed in a domain expert and then the expert opened a new MR to bring the discussion there which was discussed by the whole frontend team. * Suggested to query the component with a component selector rather than a data-testid (same here, here and here) * Suggested to remove some tests as they were testing Javascript's internal behaviours. At the end, this was a very educational MR for me because I learned how maintainers act when there are disagreements.

For this MR, I'm going to quote both the maintainer and domain expert's feedback as I believe they are very valuable:

@mrincon:

Savas referred to me as maintainer to express his concern about the original solution which used inheritance in Vue data(). There are a few things I would like to highlight here:

• Some conversations are not easy but it is important to have them, and as a maintainers we can learn from them.
• We maintainers shouldn't work in isolation, so it is always good to rely on each other when we need help or support, as we can't always be right.
• For example, Savas referred to me, and I in turn referred to @nmezzopera's expertise to check my gut-feeling that this was not an optimal solution.
• Once a "difficult" conversation is solved/concluded, it is always good to document the outcome so we can refer to it in the future.
• gitlab-org/gitlab!38470 (merged)

@nmezzopera:

@mrincon I agree with your analysis 100% in addition I am pleased to see this line from @svedova

This was an incredibly educational MR for me because I always wondered what to do in case the suggestions are pushed back by the author.

This is not a common case, but is a case that indeed takes a huge amount of efforts and energy, not always the resolution is that the maintainer(s) put their foot down and stop the MR, sometimes is the maintainer turn to acknowledge the mistake, that he learned something and move forward. That being said we must be always ready to protect the quality of the codebase, and we should never feel shy to reach out to our peers and to transparently state our worries. In this case the author was pushing strongly to move forward, so I don't think any reviewer would have been able to 'stop' this, but I appreciate @svedova continuous and constructive involvement in the conversation

Links to Non-Trivial MRs I've Written

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

So far I created 104 MRs in 8 months. I'm not going to list all of them here as It would take forever to review them 😅 Instead I'm going to list some of them which I believe make sense to review.

Some Non-Trivial MRs	Comment
Move CSS to page related bundle	This MR moves security dashboard related CSS to it's own bundle. It's still not merged as of writing but got 3/4 approvals without a comment. This is important because it taught me how to extract specific page related CSS to its own bundle.
Introduce vulnerabilities trends chart	This MR adds Vulnerability Report link to the Security & Compliance sidebar navigation and moves the vulnerability reports to under that page. It received some comments from the reviewer and maintainer but given the length of the MR I suppose this is normal 😅

Right after I joined GitLab, (I think it was during my third week) I took ownership of the Security Dashboard Migrations (from REST to GraphQL). @samdbeckham handed over the work he had been doing while he was working in the Defend team as a temporary FE Engineer. His help and this project helped me onboard pretty quick. Here are a few MRs that are related:

MRs (mostly %12.10 and %13.0)	LoC	Overview
Display the unavailable state when user has no access	+23,-14	7 comments*
Add vulnerable projects table	+32,-1	13 comments*
Add filters to group level	+54,-3	8 comments*
Load vulnerabilities for instance dashboard	+375,-3	15 comments*
Add filters to the instance level security dashboard	+55,-2	15 comments*

*The comments here contains also the shoutouts for reviews and the thanking part.

Here's the latest state of the Security Dashboards:

Security Dashboard	Vulnerability Report

List of some recent tiny MRs that were merged pretty quick
- gitlab-org/gitlab!47966 (merged) - gitlab-org/gitlab!48663 (merged) - gitlab-org/gitlab!48872 (merged) - gitlab-org/gitlab!47961 (merged) - gitlab-org/gitlab!48375 (merged)

Feedback from my Manager

Last but not least, I'm going to quote a few feedback from my manager @lkerr, during my employment:

Impressed with the small number of comments that the majority of your MRs merge with. Nice work on providing context for reviewers & implementing such clean code.

The pace at which Savas has developed new features is impressive. Even as impressive is that he’s able to do so without creating a large number of follow-up issues or unexpected defects resulting from his work. (!42347 (merged) !41092 (merged))

Savas' understanding of our product, and how customers interact with it, is demonstrated through the questions he asks and suggestions he makes on ways to iteratively deliver value to our customers.

Savas contributed well above the requested number of Pajamas migration MRs in our Q3 & Q4 team KRs, this gave him a lot of visibility into other areas of the GitLab codebase.

Your MR rate for Aug was super impressive!

AMAZING MR rate for Sept!!

Impressive progress getting through 13.3 issues. Thanks for picking up so much while other team members were out.

Nothing new: same glowing feedback about your productivity, asych communications, and impressive MR throughput.

Conclusion

All in all, I'm really looking forward to become a Frontend Maintainer and be able to help GitLab even more 🙏🏻 I hope I was able to make my case in this MR description 😅 Looking forward to hearing your feedback on this!

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

Once This MR is Merged

1. Create an access request for maintainer access to gitlab-org.
2. Create an access request to be added to 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 🤘 😎 🤘