Photo by Tomáš Malík on Unsplash
Often times after getting past a tedious pull request, you know, the ones that have so many nit-picks, I wonder - how could it have gone better?
When going into a pull request, I try to focus on one thing - let's ship something that's good enough. This usually means that I focus on things like clarity and correctness, rather than aesthetics or organization.
Something that I've started to really veer away from and dislike are comments that have to do with personal preferences.
For example, many people have the preference that in a React codebase, a file should have a single component and nothing else. Anything that is not that single component should be moved to a separate file, where it be another component or a utility.
In practice, I find that this style of coding can be useful, but when overly used it becomes a real chore. Sometimes functions or components have a single use, and to have to create a whole new file to house that purpose leads to reluctance - better inline it.
Another aesthetic preference I don't really care for is line count. When does a file have too many lines? I'm not really sure. I tend to go with instinct on this. When the tree becomes convoluted in a way that makes me feel lost, I start to break things into subcomponents. Sometimes this can have benefits in performance when done well, as state can become pushed to subtrees and reduce the amount of renders happening higher up. However, if your team likes to nit on single components per file, then this sort of refactoring can become deterred as well.
I found that happening to me a lot when working with Angular. The fact that there is so much boilerplate to setting up even the simplest of components made it so I avoided it whenever I could - which lead to really hard to read components.
In general I think intuition should be applied. If the sub-component that can be made can also provide improvements that are not just aesthetic, then you should do it.
So overall, my rules are:
- Is this correct?
- Can I provide non-aesthetic improvements?
- Is this good enough?
If so, lgtm.
Top comments (0)