Add David Fernandez as backend maintainer (GitLab)
Trainee maintainer issue: #6264 (closed)
🔭 Overview
I've been working at GitLab since October 2019. It feels like it was yesterday
As a member of the Package Team, my main area of work is:
- Package Managers APIs
- Authored the NuGet support
- Improved existing APIs
- Package APIs (rest and graphql)
- Dependency Proxy
- Integration with the Container Registry, which includes
- APIs (rest and graphql) to query the Container Registry objects
- Cleanup policies
I worked quite a bit to improve the situation regarding file uploads:
- Make workhorse transmit the upload parameters in a safe way
- Make rails support those parameters
- Update different upload endpoints to stop using
UploadedFile.from_params
with unsafe inputs. For example, the lfs controller - While at it
- Lastly, for good measure
I usually persevere if I see a bug that can be fixed. Here is an example: Add wildcard segments support to grape path helpers.
📊 Statistics
- 180 MRs authored
-
133 MRs reviewed
- Now, you can see that I did not a great job to track these in my trainee maintainer issue
😈
- Now, you can see that I did not a great job to track these in my trainee maintainer issue
-
Proud author of MR 44444
😺
🔍 Examples of reviews
- Use the Dependency Proxy with private GitLab groups: #6264 (comment 477520172)
- A quite complex MR involving several endpoints and authentication
- A good example where I knew that several pair of eyes was needed on this one
- Dependency proxy caches the manifest: #6264 (comment 467275542)
- Gave several suggestions to improve code readability
- Noticed that a
Tempfile
was used but not properly closed and deleted
- Define a new home page for pipeline authoring: #6264 (comment 454006591)
- When adding a change that is gated behind a feature flag, make sure that specs cover the
feature flag enabled
andfeature flag disabled
conditions.
- When adding a change that is gated behind a feature flag, make sure that specs cover the
- PyPI: API skeleton and authentication #6264 (comment 313674451)
- Gave several suggestions to improve code organization
- Suggested a security review as this was adding new endpoints that require authentication
I was also involved in reviewing non straightforward community MRs for my team such as this one.
📈 Things to improve
During my past time at GitLab, I tried to see/explore different sides of the codebase as much as possible. That is what motivated me to work on file uploads. It is a transversal feature.
Having said that, there are areas that I don't work often with, for example, the view layer and the rails controllers. I'm ok with small / minor changes because in my past positions I worked extensively with erb
, haml
, stimulus
& friends. Sidenote: sometimes, I wish the Grape API controllers to be standard rails controllers to avoid this kind of issues.
I think my biggest fear as a maintainer will always be to accept a change that breaks a feature. I still think this risk is part of the game and can't be removed entirely. As such, I accept it and try to minimize it as much as I can.
That's why I'm always interested in issues that make me discover other parts of the codebase than my day to day work (my work on file uploads are a good example here).
Having said that, my aim is not to know all the GitLab (rails) parts (I'm not sure if that is even possible
Also, I know that it's always possible to pass the final review to domain experts. I would even say that it should be mandatory when the MR complexity is higher than usual or security aspects are involved.
@gitlab-org/maintainers/rails-backend please chime in below with your thoughts, and approve this MR if you agree.
Developer checklist
-
Before this MR is merged -
Mention @gitlab-org/maintainers/rails-backend
, if not done (this issue template should do this automatically) -
Assign this issue to your manager
-
-
After this MR is merged -
Request a maintainer from the #backend_maintainers
Slack channel to add you as an Owner togitlab-org/maintainers/rails-backend
-
Consider adding 'backend maintainer' to your Slack notification keywords
-
Manager checklist
-
Before this MR is merged -
The MR has been open for 5 working days -
More than half of the existing maintainers approve the MR -
There are no blocking concerns raised (if there are, please follow https://about.gitlab.com/handbook/engineering/workflow/code-review/#how-to-become-a-project-maintainer)
-
-
After this MR is merged -
Announce the good news in the relevant channels listed in https://about.gitlab.com/handbook/engineering/#keeping-yourself-informed
-