I think you got called into the office coz you were being a dick!
Goodbye, Clean Code (2020)
181–190 of 224 posts
Re: Goodbye, Clean Code (2020)
#182Blindly checking out code late at night, without PR review, without discussion about context, overwriting approved and (I assume) tested changes, for code you did not write and were not included on review for, BLINDLY merging to master (!?) - this could be tantamount to a fire able offense depending on where you work. I would have a hard, long talk with any of my mid or junior engineers if this was attempted (although master should ideally be protected, preventing this scenario unless OP had administrative senior permissions on the code base).
All around a mess. I agree with the sentiment (write clean code unless you shouldn’t) but there is some serious process issues here that should be caught and corrected and would have made this entire issue something during the PR review phase, where OP presumably asks for an improvement and is answered with the context for the decision. Not to mention the red flag of “I know better than my peers” attitude of many programmers I have known in the past, leading to distrust and backstabbing. It can get ugly.
Re: Goodbye, Clean Code (2020)
#183Earlier quoted context omitted.
CI is more than that, at least in my understanding of the term. You have to have some sort of automated testing before doing the merge. We were doing the same shit (all pushing to the same branch) and it broke constantly before we introduced some tests and merges only after a successful test run on a separate CI server. Just pushing everything into the main branch without any checks is no CI, even if it technically c…
That's `automated CI` which is now or less interchanged with just `CI`, however at face value there is no requirement that all CI be automated, it is just usually better to do so.
For anything non-trivial (read: more than around 10-20k SLOC of C or equivalent; straightforward with minimal branching logic; non-critical) you cannot, in any practical sense, have CI without automation. Unless, of course, you want a garbage system. If you do the minimum things necessary to ensure the system is correct (in both verification and validation senses) and of decent quality, and you have no automation, you will not have CI because you will introduce delays between code change and deployment ranging from days (for smaller projects) to weeks or months (for significant projects). That delay means you do not have CI. And it also encourages batching many changes together so that you only have to have one test run (or a small number of test runs) which is also the opposite of CI.
Now, you could bypass all those tests and reviews and just deploy it anyways. And then you'll have a shitty codebase, but you'll have CI so that's good, right?
Re: Goodbye, Clean Code (2020)
#184Duplicated code != Messy code. The only time duplicated code is bad is when there is an actual logical requirement for the multiple instances of duplicated code to be the same. Like some actually underlying concept linking the duplicated code sections that is worth abstracting. That is not always the case. Just as often IME the situation is actually, "these two things happen to have the exact same behaviour right now…
I don't think this is necessarily true. If I have one place in my code where I sign unsubscribe tokens for emails and another place where I sign cookies for auth I don't want to implement a HMAC twice. Even if the verification paths are different so the algorithm doesn't actually need to be the same it is still better to use the same code for both so that any bugs, inefficiencies or other problems can be fixed once r…
Yeah, you're right and I think we agree here. I should have been clearer about that.
Re: Goodbye, Clean Code (2020)
#185The real story here is not about the code. When it's your own personal project and you can be your own little tyrant, do whatever you want. Be as "clean" as you need to be. But this was at work, and Dan was being a bad coworker. He snuck in a change in the middle of the night over a coworker's code. This code wasn't his responsibility. He didn't leave his thoughts on a PR where others could discuss it. He overwrote s…
Agreed, 100%. I would be beyond livid if a coworker did this to me if I were the original committer. OP does not realize the insane level of mistrust he just created with this action.
Re: Goodbye, Clean Code (2020)
#186See also: “Reuse Versus Compression” chapter from the book “Patterns of Software” by Richard Gabriel.
Re: Goodbye, Clean Code (2020)
#187I'm shocked that from all discussion nobody noted the root of all evil: "It was already late at night (I got carried away). I checked in my refactoring to master and went to bed, proud of how I untangled my colleague’s messy code." No PR, no code review, no CI. Just a cowboy pushing to the master..
And... "it was already _late at night_" (emphasis mine). I learned a long time ago that not all hours in a day are created equal.
Re: Goodbye, Clean Code (2020)
#188Earlier quoted context omitted.
That's `automated CI` which is now or less interchanged with just `CI`, however at face value there is no requirement that all CI be automated, it is just usually better to do so.
For anything non-trivial (read: more than around 10-20k SLOC of C or equivalent; straightforward with minimal branching logic; non-critical) you cannot, in any practical sense, have CI without automation. Unless, of course, you want a garbage system. If you do the minimum things necessary to ensure the system is correct (in both verification and validation senses) and of decent quality, and you have no automation, yo…
I'm not advocating for this from an engineering perspective, just stating that it does meet the semantic root of `CI`, regardless of whether you believe it meets _your_ threshold of "good" CI.
Re: Goodbye, Clean Code (2020)
#189Earlier quoted context omitted.
Personally, I disagree that DRY is the same as “clean”. Clean code is merely code that has been thoughtfully structured (regardless of repetition) and is relatively transparent as to its purpose.
I've seen a few developers see multiple "duplicated" API formats for instance (for different API's) in the same codebase and think those are "duplicated" and then entangle two completely separate API's together. This is worse IMO
There is accidental duplication to beware of. That is when different things looks the same. Then there is real duplication.. Only the later is bad and should be eliminated. Telling the differences is hard
Re: Goodbye, Clean Code (2020)
#190Earlier quoted context omitted.
IME abstractions generally begin with the latter case. Perhaps there's one or two corner cases that the abstraction also covers, but it seems justifiable at the time because the core of the code really ought to work the same across all cases. Then you slowly start adding corner cases, or you change your new feature slightly so that the abstraction has to change. Little by little you end up with this insanely complica…
An issue in the duplicated approach is keeping track of the different blocks once they diverge enough. In the best case scenario they all slighty change over time without forcing complexity on each other. But it becomes a problem as changes that should be breaking only break part of the code, and the rest can go unfixed as nobody remembers all the linked bits. It would be critical for instance if the duplicated bits…
I'm not necessarily claiming that all abstractions are bad. I'm say that (as a general rule) engineers should be more willing to duplicate code.