Add Deepika Guliani as a frontend maintainer
Manager Justification
It's hard to specify hard requirements for becoming a maintainer, which is why the documentation consists of flexible guidelines. Reviewers are encouraged to think of their eligibility for maintainership in the terms of "I could be ready at any time to be a maintainer as long as it is justified".
-
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.
@deepika.guliani has been a reviewer of the GitLab frontend codebase for almost a year and a half and has been officially training to become a maintainer for over three months. In that time, she has approved over 280 MRs. She has also been mentored by @anna_vovchenko. As evident from the details below, Deepika is a thorough and skilled reviewer and will be an outstanding addition to our group of skilled frontend maintainers.
Click to see Deepika's extended application
| Welcome to Deepika Guliani's maintainer application |
|---|
| Hey |
For more detailed information visit the trainee issue - Trainee FE Maintainer (Gitlab) - Deepika Guliani (#34345 - closed).
Out of the 280+ MR's I have reviewed till date , 7 out 10 MR's were merged as is or with non-blocking suggestions from the maintainer.
My review philosophy
- test, test and test - I like to test out the MR first hand and see if it working as expected. In case there are some issues in verifying it, I try on the review app otherwise best to ask the author and clearly mention that I could not test it locally and it is always good to mention any use case that could not be tested . Examples - 1, 2, 3, 4
- Ask questions. Whenever in doubt about something that is not clear , I always ask and do not assume anything and be on the same page as the author. I like to confirm and ask for UX approval of an MR if there is any doubt in appearance or text copy. Examples -1, 2, 3, 4
- a mixture of "don't make assumptions" & "assume positive intent" & "radical transparency". Communicating is hard especially async communication. I try my best to be very clear and am quick to provide a demo-video or something similar to make sure nothing gets lost in between the lines. Examples - 1, 2, 3, 4,
- leaving praise doesn't hurt and should be done more often. Examples - 1, 2
Links to Non-Trival MRs I've Reviewed
| MR | Changes | Weight x/10 | Notes |
|---|---|---|---|
| Support pagination when loading award emojis in... (gitlab-org/gitlab!125121 - merged) | 344-104 | \textcolor{red}{\text{10}} |
This was an MR which was really interesting to review since it tested basic apollo concepts and meant confirming scenarios in my local environment and having some sync discussions with the author. Good collaboration with author and subsequent discussions with the maintainer. |
| Add "Closed Issues" to Issue Analytics chart (gitlab-org/gitlab!130784 - merged) | 1028-48 | \textcolor{red}{\text{10}} |
Multiple questions and rounds of suggestions in this one. Pointed out not to use an apollo context because of globally enabled FF. Merged with very minor suggestions of maintainer. |
| Rename files in pipeline mini graph, stub out n... (gitlab-org/gitlab!126773 - merged) | 830-591 | \textcolor{red}{\text{10}} |
Asked a lot of questions in this huge MR since it was mostly restructuring but meant we had a chance to better our codebase. In addition to creating some follow up issues I also differentiated the blocking suggestions and nitpicks from the non-blocking ones so that it is easier to move the MR forward. |
| Add Tracing List UI (gitlab-org/gitlab!125572 - merged) | 3377-134 | \textcolor{orange}{\text{8}} |
This MR was a little tricky to review since this was based on some backend changes which were not merged. Had to verify the MR locally with lots of questions with more than a couple of rounds of review and being merged in the nick of time for the milestone cut off. |
| Apollo boards - Error handling (gitlab-org/gitlab!125661 - merged) | 1320-301 | \textcolor{orange}{\text{7}} |
Caught a UI bug with loading indicator when the apollo query fails and pointing out that we need to disable the isLoading bool in the finally code block |
| Migrate MR «more actions» dropdown to Vue (gitlab-org/gitlab!117298 - merged) | 762-249 | \textcolor{orange}{\text{7}} |
Another MR with discussions and questions to the author. Pointed a typo error and also provided some patches to the author when he asked for help. |
| Add to do widget for work items (gitlab-org/gitlab!118135 - merged) | 471-4 | \textcolor{orange}{\text{7}} |
A couple of review rounds with multiple suggestions and questions saving some UX inconsistencies. |
| Aggregate multiple abuse reports by the user & ... (gitlab-org/gitlab!119282 - merged) | 568-231 | \textcolor{orange}{\text{6}} |
Took some time to setup ( both frontend and backend set up steps for data ). Merged with 1 minor suggestion from maintainer |
| Notification widget for work items (gitlab-org/gitlab!115511 - merged) | 310-10 | \textcolor{orange}{\text{6}} |
Suggested multiple changes to follow the code review guidelines and even anything that was not yet documented but announced on slack. Required good knowledge from GraphQL and apollo. Had a sync discussion with the author as well |
| Hook up update schedule mutation (gitlab-org/gitlab!124783 - merged) | 536-58 | \textcolor{orange}{\text{5}} |
Some suggestions for spec coverage and refactor to follow code review guidelines. Merged with no additional feedback from maintainer |
Here are the community contributions who took some perseverance as an MR coach
Community contributions require more effort from the reviewer to make sure there is clear communication, verbose suggestions with patches and clear guidelines so as to the code adheres to the GitLab code standards. This also gives us an opportunity to explain things at the root level and answer a lot of other questions that the contributors have. The below examples showcase that I have the patience and technical expertise to communicate clearly with external developors.
| MR | Notes |
|---|---|
| Migrate new issue GlDropdown to GlDisclosureDro... (gitlab-org/gitlab!127996 - merged) | Multiple discussions with the author to make sure the migration works as expected and not introduce anything new |
| Feat: Add print to PDF for wiki (gitlab-org/gitlab!125260 - merged) | Created a patch for the contributor to give an example of the suggestions with also going through multiple discussions with the author |
| Add no hard coded url eslint rules (gitlab-org/gitlab!125196 - merged) | Suggested to make changes in the affected files rather than just add eslint-disable rule |
| Resolve "Text is truncated on long description ... (gitlab-org/gitlab!118424 - merged) | Requested for UX review as well in addition to giving review comments and giving examples for the same |
| Migrate "work_item_note_actions.vue" to GlDiscl... (gitlab-org/gitlab!118420 - merged) | Required multiple rounds of communication with the author to get the end result |
| Pass branch sort order to drop down component f... (gitlab-org/gitlab!108403 - merged) | Helped the author on how to pass a backend parameter to the frontend by creating a patch |
MR's with a larger goal which I have been a reviewer consistently
- Epic - Move from vuex to apollo in boards. Here are the list of 27 MRs where I gave some valuable feedback while in the review phase
- Multiple UX paper cuts MRs
Andd.... I've also reviewed these!
Smaller Reviews with No Maintainer Additions
Things I worked on and take pride in
- Accessibility scanner dashboard POC - https://gitlab-org.gitlab.io/frontend/playground/accessibility-scanner/
- Working on a number of migrations along with project work
Challenging situations
When I introduced a bug and I noticed it before it hit production
| TLDR; |
|---|
|
While working on my first page_bundle_migration issue , I noticed a regression soon enough after it was merged but before it hit prod. I proactively created a follow up MR and asked in the slack channel if any frontend maintainer available to merge it. By the time the MR was merged(and the original MR hit production) some other regression was notified in the is-this-known channel and it was decided to revert the MR on the same day to mitigate any more harm The silver lining out of this incident was ended up moving the changes behind an FF with the help of pairing session with Leipert and making sure any future incidents of page bundles migrations are solved only with an FF turned on or off Gitlab values showcased in the incident |
When I broke master ( two times )
| TLDR; |
|---|
|
I was working on this issue which was a part of the Beautifying the UI 15.6 and was a spill over from the milestone since other issues/MR's had taken precendance. It took me a little time to start working on it 100%. Turned out to be so much bigger scope than I had expected and quite a large MR . This was one of the big MR's outside of only the project work that I was doing and took me sometime to fix the specs and was over the moon to get it merged. But only to get it reverted , not only once but twice. The reason was that the FF behind which the changes were done was on for some groups. I learnt the following lessons from this incident
Gitlab values showcased in the incident |
Before Merging (Manager Tasks)
-
Close any relevant trainee maintainer issues with a comment indicating that this merge request is being created, as (they are no longer required to become a maintainer). -
Mention the maintainers from the given specialty and ask them to provide feedback to the manager directly. -
Leave this merge request open for 1 week, to give the maintainers time to provide feedback. -
Ensure we have at least 2 approvals from existing maintainers.
Once This MR is Merged
-
Create an access request for maintainer access to gitlab-org/<project>. -
Join the [at]frontend-maintainersslack group -
Ask the maintainers in your group to invite you to any maintainer-specific meeting if one exists. -
Let a maintainer add you to gitlab-org/maintainers/frontendwithOwneraccess level. -
Announce it everywhere -
Keep reviewing, start merging 🤘 😎 🤘