Make Sarah Groff Hennigh-Palermo a Frontend Maintainer
Summary
🐒 💝 𝓗𝐞Ł𝐥ό ⓔＶ𝓔ｒ𝐲𝐎𝓝€ ☟🍫
It is the time we all have dreaded anticipated. That's right — I am opening my maintainer MR. We may not always agree about test structure, but I think you'll find within the rollicking adventure below some good catches, some solid architecture suggestions, more d3 and data parsing than you expected, and at least one automated script that did not break our CSS.
Don't make me cry — smash that approval button.
★·.·´¯·.·★ ⓢᗰＡ丂卄 เт ★·.·´¯·.·★
Links to Non-Trival MRs I've Reviewed
Big Chonks (Large-ish, refactors, architectural suggestions)
| MR | Changes | Link | Comments |
|---|---|---|---|
| Coverage fuzzing db security dashboard (Frontend) | 639-24 | gitlab-org/gitlab!36011 (merged) | A large-ish, complex review for a backend engineer taking care of frontend changes. Involved helping reviewee understand some of our norms in a portion of the codebase that is itself particular. We also had to break out the backend to avoid mixed deploys, which became known as an issue during review. Few additional maintainer comments. |
| Refactor pipeline form to Vue | 512 -36 | gitlab-org/gitlab!35674 (merged) | Got a shoutout from reviewee. Covers refactoring data-structures to be more straightforward and less prone to index-change issues, as well as some Vue style norms. |
| Display No Environments for Feature Flag Strategies without Scopes | 257 -68 | gitlab-org/gitlab!37007 (closed) | Still open and may not even merge. But! I have included it because I think it is a good example of how I work with authors to untangle complex code and uphold testing standards, while also considering the tradeoffs between |
| DAST Site profiles - Form MVC - Create the basic form - Frontend | 459 -5 | gitlab-org/gitlab!36355 (merged) | Covers collaboration with design & writing to maintain tone; some structural suggestions; some tests; some GraphQL. Maintainer had nice suggestions but was also prepared to approve as-is, so win? |
| Show loading in MR pipeline widget when no pipelines | 291 -238 | gitlab-org/gitlab!35980 (merged) | Some conversation on icon accessibility. No additional maintainer changes. |
| Feat(charts): add general skeleton loader | 480 -0 | gitlab-org/gitlab!35012 (merged) | SVGs are my passion and this review had them in spades. In the process of helping the author move away from hard-coded magic numbers into something a little more responsive, we also learned a lot about GlSkeletonLoader and clipPaths. Few maintainer comments. |
Include uploadedFiles param in the snippet Create mutation request |
183 -52 | gitlab-org/gitlab!34221 (merged) | Felt cute. Made some structural suggestions. Also touches on GraphQL a little. |
... and, the biggest Kahuna of them all, a very large MR that I, using only peer pressure and the threat of disappointment, convinced the author to break down so we could have an even more thorough review
| MR | Changes | Link | Comments |
|---|---|---|---|
| The Original: Allow Users to Create and Rename User Lists | 78-84 | gitlab-org/gitlab!36598 (merged) | This is the original and was at 1085-108. It came back around to be the smaller final MR. |
| Add Store for Creating a User List | 153 -0 | gitlab-org/gitlab!37005 (merged) | No further maintainer changes. |
| Add Components for Editing and Creating User Lists | 562 -0 | gitlab-org/gitlab!37012 (merged) | Some structural suggestions, including one of which was echoed by the maintainer. A few maintainer comments on tests, but nothing major. |
| Remove Deprecated Button Usage | 17 -16 | gitlab-org/gitlab!37000 (merged) | Did not re-review this one; it went straight to maintainer. I think we can all see why. |
| Mount Vue Components for Creating and Editing User Lists | 101 -37 | gitlab-org/gitlab!37013 (merged) | Recommends provide/inject pattern. Also has few additional maintainer comments. |
Clean Cats (Few to No Maintainer Additions)
| MR | Changes | Link | Comments |
|---|---|---|---|
| Mark diff files if they're part of broken symlinks | 154-5 | gitlab-org/gitlab!37746 (merged) | Actual quote: @sarahghp this was an awesome review! |
| Resolve "Add no graph empty state for DAG" | 197-9 | gitlab-org/gitlab!35053 (merged) | One small miss on template syntax |
| Disabled Edit button for binary snippets | 67-27 | gitlab-org/gitlab!30904 (merged) | Has a nice catch and no further maintainer comments. |
| feat(css): Generate stateful utility classes | 108-14 | gitlab-org/gitlab-ui!1178 (merged) | No further changes. |
| Geo Package Files - Fetch Data Correctly | 262-144 | gitlab-org/gitlab!33522 (merged) | No further changes. |
| Show User List Details | 475-26 | gitlab-org/gitlab!35369 (merged) | This is pretty big and has just two nits, so I think it deserves to be here. |
•´¯•» 🎀 𝒶𝓃𝒹 𝒻𝒾𝓃𝒶𝓁𝓁𝓎 🎀 »•¯´•
🏆 a weird one that I am just proud of 🏆
| MR | Changes | Link | Comments |
|---|---|---|---|
| Update dependency @gitlab/ui to ^9.23.1 | 5-5 | gitlab-org/gitlab!26863 (merged) | Although it appears to be a mere chore review, this change revealed some assumptions we were making about Gitlab UI workflows and ultimately resulted in conversations and clarifications about the best ways to develop iteratively on a library with this sort of release process. Check out the comments in the trainee issue. |
Links to Non-Trivial MRs I've Written
DAG Visualization (gitlab-org/gitlab#215517 (closed))
These MRs together are what made this 70s–style tubing possible.
| MR | Changes | Link | Comments |
|---|---|---|---|
| Refactor pipeline_details_bundle.js to break out functions | 38-27 | gitlab-org/gitlab!32438 (merged) | In iterating, it is important to prepare the ground. |
| Add DAG endpoint and graph shell | 170-4 | gitlab-org/gitlab!32460 (merged) | No changes, but one small regression. Learned a lot about considering HAML and now I inquire about it in every review I do. |
| Add DAG data parsing utils | 416-5 | gitlab-org/gitlab!32768 (merged) | A handful of comments, mostly about destructuring. Fewer regressions. |
| DAG Visualization: Return of the Graph | +1213-42 | gitlab-org/gitlab!32890 (merged) | This was large, although it was mostly moving & productizing code from the proof of concept. Some interesting discussion about Vue + d3. A few comments from the reviewer and one from the maintainer. |
| DAG MVC: Refactor the Graph! | 274-240 | gitlab-org/gitlab!33401 (merged) | Reining in some excesses. No maintainer changes; a few reviewer comments. |
| DAG MVC: Interactions | 303-23 | gitlab-org/gitlab!33832 (merged) | No maintainer changes; a few reviewer comments. Some cute tests, if you're into that. |
| DAG MVC: Beta Link, Documentation, Feature Flag | 80-8 | gitlab-org/gitlab!33958 (merged) | Working with design and tech writing to make the feature available to all. |
| Add annotation component for DAG | 421-37 | gitlab-org/gitlab!35240 (merged) | Very few comments from reviewer and maintainer. |
| Convert DAG to Use GraphQL | 503-149 | gitlab-org/gitlab!38085 (merged) | The Verify::CI team is moving to GraphQL and the DAG is one of the first segments to be updated. This MR is still in progress, as is the backend it depends on, but I thought I'd include it as a cool bonus. (Change numbers may change.) |
AWS Dropdown ↔ ️ GlFormCombobox (gitlab-org/gitlab#26777 (closed))
This project added a combobox component, first in Gitlab for product scheduling reasons. It was then backported to Gitlab UI and replaced in the product where it originated.
| MR | Changes | Link | Comments |
|---|---|---|---|
| Resolve "Typed AWS environment variables for access keys & region" | 675-36 | gitlab-org/gitlab!29124 (merged) | Adds the component initially. It had good bones that were refined with help from the reviewer and maintainer. Likewise the tests — learned our newest (at the the time) async test approach and wrote tests that withstood a decent restructring. |
| feat(GlFormCombobox): Add GlCombobox component | 524-0 | gitlab-org/gitlab-ui!1549 (merged) | Moves the component to Gitlab UI. A handful of comments in a very thorough review; most minor. Learned more about what we can await on that side of the divide. |
| Replace CiKey field with GlFormCombobox | 7-420 | gitlab-org/gitlab!37091 (merged) | Closes the circle. Discovers a way to break an e2e test without the lint job failing. |
Ongoing CSS Replacement (gitlab-org/gitlab#36857)
This project covers making it possible to use Gitlab UI utility classes in Gitlab. It is not finished and these MRs don't look gigantic, but sometimes the biggest changes are made in the smallest pieces.
| MR | Link | Comments |
|---|---|---|
| Add first draft of revised CSS documentation | gitlab-org/gitlab!31963 (merged) | Tell em what you're gonna do. |
| Add rules for utility CSS to Danger | gitlab-org/gitlab!30501 (merged) | Tell em again, but this time in Ruby. |
| Deprecate Gitlab UI class-name clashes | gitlab-org/gitlab!27610 (merged) | Do it with a script. |
| Remove classes that are duplicated in Gitlab UI | gitlab-org/gitlab!31863 (merged) | Do it. |
| Include Gitlab UI utilities | gitlab-org/gitlab!30724 (merged) | Really do it. |
| Remove shim classes | gitlab-org/gitlab!32874 (merged) | Cleanup. |
| Remove outdated shim notes | gitlab-org/gitlab!31967 (merged) | Clean it again. |
Auto-stop (gitlab-org/gitlab#20956 (closed))
| MR | Changes | Link | Comments |
|---|---|---|---|
| Resolve "Auto stop environments after a certain period" | 403-65 | gitlab-org/gitlab!20372 (merged) | Features design edge-case detection and a handful of maintainer and reviewer notes. |
| Fix two bugs in environments table | 19-16 | gitlab-org/gitlab!20737 (merged) | Addresses bugs found in the course of the previous work. |
| Refactor environment table titles and spacing | 475-26 | gitlab-org/gitlab!20769 (merged) | Likewise addressed some tech debt discovered in the course of working. |
@gitlab-org/maintainers/frontend please chime in below with your thoughts, and approve this MR if you agree.
🎅 🐊 รＭ𝓐ˢʰ 𝓣𝐡𝐚ⓣ 𝓫Ữᵗᵗ𝕠ᶰ 🍔 🐯
Once This MR is Merged
-
Create an access request for maintainer access to gitlab-org. -
Create an access request to be added to the [at]frontend-maintainersslack group -
Let a maintainer add you to gitlab-org/maintainers/frontend -
Announce it everywhere -
Keep reviewing, start merging 🤘 😎 🤘
Edited by Sam Beckham