An Early Lesson
In my early days of professional programming I ran into a bug. The open function was executing unexpectedly and closing the dialog unexpectedly, or the close function was executing and opening. Something like that. So I learned to setIsOpen(false)
or setIsOpen(true)
, but never setIsOpen(!isOpen)
. I always let other devs setIsOpen(!isOpen)
though. It's really not a high risk thing.
Over-Engineering Something Minuscule
So much later I felt like doing type first development on the silly thing just for kicks. This was my TypeScript type definition that acted as contract to hide internal logic, even if it's overkill. (It is overkill and I always knew it.)
type IsOpen =
| {
isOpen: true;
close: () => void;
}
| { isOpen: false; open: () => void };
Does this force you to write an if statement or ternary expression to access the close or open function, yes it does. Is that ridiculous since calling closed when it's already closed (or open when it's already open) harmless? Of course. Knowing this did I commit it and let it go to production, yes I did.
Impediments
But am I a little impatient and not understanding now that somebody has objected to past over-engineering and impeded my progress on an unrelated pull request that was limited to fixing TypeScript errors and adding scripts to aid in development? Yes. The PR is intended to be a minimal one to aid me in the development of all subsequent 50 PRs. This low risk PR has already passed QA twice with 3 separate sessions of QA and refactoring the code has now required an unnecessary 4th QA session.
I have been aggregating minimal changes required to write unit tests and pass quality gates for 2 months and 2 weeks, unable to get PRs reviewed, approved, tested, and finally merged. 4 weeks ago I switched to a different repository and created over 50 micro PRs. It didn't help. I have previously been coding for about 10 years and this has never happened to me.
Advice
When reviewing PRs, make as many things optional as possible. Start your comment with "Optionally, ". If you feel like it has to be changed before merging, offer to create a PR into theirs upon request. If you feel like it has to be changed but can wait, it can probably wait until you or another developer thinks the same thing and is touching it. Always state clearly that something is or isn't required for PR completion and precisely what is required. But above all, be flexible enough to allow a PR with no runtime risks to be completed.
Also, in my not so humble opinion, look into continuous delivery until you become convinced it can work and it can work well.
Top comments (0)