Maybe it’s just me but having unfinished, dead, or scratch code in a production codebase really annoys me. Either finish your work or delete the unneeded code. More than a few times I’ve sunk time out of my day into investigating some code path only to realize it’s completely unused.
The solution ended up being a rather simple feature flag. Within a few days, he coded a flag that would enable not-inlinging; and updated our test infastructure to look for regressions on unit tests with the flag enabled. Going forward, developers we responsible for making sure their changes didn't cause regression when the flag was enabled; and everyone was able to slowly chip away at everything the feature broke when they had spare cycles.
What would have been a major stop-the-world refactor with our most senior engineers, turned into a slow moving non-issue.
[0] Lack of turing completeness was and remains an explicit design goal, so recursion was explicitly forbidden.
As for experimentation, I like percentage rollouts with segregated control/treatment group metrics. I agree that trivial on/off flags should be replaced by code deployments where possible (at my day job, code deployments happen to be a lot slower).
The exception is experimental work. Obviously experiments aren't being merged into master, so they don't need to be included in the arithmetic for any processes that eventually involve master.
Yeah, you might be really responsible and rebase and test your feature frequently, but what about other people? They are more likely to break the integration with your code since they can't even see it.
> Either finish your work or delete the unneeded code.
It's work in progress. We're working on getting it finished.
So finish it and then I will merge your PR ;) What's the use of putting it in master if it's not finished?
It's the author's responsibility to get it merged successfully. If they're taking too long and have to rebase and re-work their code to integrate, that's on them. Pushing it into master is either wasting a reader's time (per the original comment) or, worse, inviting an uninitiated collaborator to use it and cause an incident.
Since you're here, I'll ask you. What do you think 'integration' means in this context?
If CI passes on your branch but later fails due to lagging behind master, it is on you to get it working before re-requesting review.
PS: The HN guidelines clearly state "Be kind. Don't be snarky." (:
In practice, I have typically seen CI done with either feature freezes, or release branches to allow creating versions where everything is complete.
That’s the problem, which I already stated before the question. That’s not integration, that’s a build. Integration is testing your changes with all the code around it.
Specifically, it’s about testing your changes with changes that started after you began writing your code.
Only small changes land in anything like chronological order. On real codebases, even with low to moderate coupling, the rug can get pulled out from underneath of you without you even noticing until something bad starts happening. CI is about exposing your code to feedback at the earliest possible moment, so you 1) don’t continue to build on violated assumptions, 2) you do lots of small merge operations instead of one big one and 3) the thoughts that began the trouble are still fresh in your head. Consequences of decisions become abstract over time. Nobody changes their behavior based on 1 year old bugs found in their code.
Long lived branches are a crutch. It’s avoidant behavior. If you can’t figure out how to make your code work with everyone else’s, why do you think waiting longer will make things better? It doesn’t. The mess just gets bigger. I’ve seen it over and over again. The guys who won’t merge early are full of delusions about their own work that doesn’t match up with the bug and incident count. They call it bad luck.
Because of the thing I said in the sentence before the one you quoted.
> If they're taking too long and have to rebase and re-work their code to integrate, that's on them.
No, in the organisations I build, we work as a team and if someone is taking too long it's because we don't have the systems in place to help them work faster. There is very clear research on the benefits of working in small batches and integrating continuously. If that research has passed you by then I strongly suggest you go back and take a look.
Visibility, and avoiding duplicated effort. What's the use of keeping it out of master?
> Pushing it into master is either wasting a reader's time (per the original comment)
The new parts they'll have to read sooner or later, and for the parts that are changing it's better for them to read the new version than the old version.
> or, worse, inviting an uninitiated collaborator to use it and cause an incident.
Why would that cause an incident? Having someone see the "upcoming" code and realise they can reuse it in something they're working on is the ideal outcome.
If the code is toggled off for a ticket in progress, great. If it’s toggled off for an epic in progress, okay. If it’s toggled off and the epic is complete/abandoned, then it’s not dark code, it’s dead code. Fire up the chainsaws.
The old code behind a toggle should be deleted before the feature is Done. If it isn’t then someone screwed up.