I work fairly strictly off of GitLab TODOs. This means if you would like me to see something, you need to mention me
@sabrams. Otherwise, there is a chance it will get lost in my emails.
I love to learn. It's my favorite part of being a software engineer, the learning never stops! In that same regard, I love to teach, so you might find that I use my comments to explain things in a way that teaches (often times it's a way for me to prove I understand a concept to myself).
My top priority is always to unblock others. If for some reason I'm unable to or need to prioritize something else, I will try to inform you as soon as I am able to. There is great power in keeping a machine running versus making sure my single part is in good shape.
I'm a strong believer in the GitLab CREDIT values. They often inform my daily decisions and workflow.
I review both database and backend code at GitLab. Although I have a different approach for each situation, I follow a general process:
- I read the description. The first thing I do is try to understand why the change is being made and what it does. A good MR description and comments from the author can turn this from a long task into a few minute task.
- Really though, I truly appreciate a good description that explains with the assumption I've never even heard of that area of code or feature before (GitLab is a big application, so this might be true!).
- I do a brief scan of the files changed and overall scope of the change to make sure it makes sense and is not trying to do too much.
- I check if everything is covered by tests?
- Are there any security implications?
- Are there any performance implications?
- I look out for code smells and other design/refactor opportunities.
- Tied into the last item, I ask myself "is it maintainable?".
- I pull the code and test it myself.
I start messy. Depending on the problem, I might just brute force a solution and then start writing tests and refactoring from there. Sometimes I will start with some well defined tests to guide a more complicated change.
I never leave unpushed code on my computer. This means that most of the time, my MRs are not in a reviewable or pristine state. I mark these as draft, but always am appreciative of early feedback on my approach.