[toraobo]

> We were lulled into letting him make key architecture decisions in isolation, because no one else was awake or online at the same time to discuss them.

That sounds like a process rather than schedule issues: important architectural decisions shouldn't be performed "on the spot" but only after some sort of discussion. Preferably in writing.

> If he refactored the codebase in the middle of the night to make his job easier, the other team members were all forced to work around his refactor.

Don't you do code review? People shouldn't be able to push code without another person looking over it.

These issues would also apply to remote or off-shore workers.

[Aeolun]

If you are the lead you generally have permission to push directly to master. This isn’t always something you do, but it’s occassionally helpful to clean up the last mess before a release.

[gregmac]

There's a world of difference between "clean up" and "refactored the codebase in the middle of the night ... [and] other team members were all forced to work around his refactor and waste time updating their PRs to match his work".

"Clean up" would be things like fixing typos/docs, bumping version numbers, fixing a build script because some path/version/etc of some external dependency changed, and sure -- mostly that's fine. If you're doing proper CI, there should be little to no surprises like this in your master branch, because you shouldn't be merging branches that break the build, and the "final release" build is really no different from any other build, other than you label it "release" (or "GA" or "RTM" or whatever) after the fact.

Refactors and any thing that could be classified as an "architectural change" really must go through code review. If someone thinks their skill is so great that reviews are pointless -- I absolutely guarantee in 100% of cases that they're gravely mistaken.

If the rest of the team can't understand the changes, well, they're bad changes, full stop. This is the team working on the code base, they are the ones that have to be able to understand it. This can be fixed of course, by firing everyone and getting a better team (just kidding), helping raise the skill level of the team, or by simplifying the changes. In my experience though, if no one understands it's usually a sign of overly-complex and/or under-documented code.

Code reviews are a two-way street, and learning is not just for the reviewee -- it's a great opportunity for the reviewers to learn, too. They get to see how someone else solved a particular problem, with rationale, the opportunity to ask questions (and 80%+ of the time, a question about how/why is usually an indicator a comment in the code is missing), and "current context" (eg: it's a change that's probably been discussed in the days/weeks before).

If the changes really are good, and it's a good PR (eg, not a massive, surprise major refactor that touches thousands of lines of code) then the PR will pass easily.

And seriously, if you need to do a massive change like this in a rush right before a release, then the team probably missed something major and the release simply isn't (and was never) ready. Major refactors means brand new code, and putting brand new code into a release with no testing (not even dev testing!) is a terrible idea.

[yjftsjthsd-h]

Sure. Not ideal in theory, but in practice that's generally reasonable. But if it's a regular thing, it should get called out and addressed one way or another (i.e., even ignoring the pathology that this conversation covers, if there's always a mess to fix right before release, then your process needs to be revisited).