Successful build - Chance fights ever on the side of the prudent.

Linter detected issues:
Executing section Default...
Executing section Source...

source/tools/atlas/GameInterface/Handlers/CinemaHandler.cpp
|  33| #include·"maths/Vector2D.h"
|    | [MAJOR] CPPCheckBear (syntaxError):
|    | Invalid number of character ({) when these macros are defined: ''.

source/tools/atlas/GameInterface/Handlers/CinemaHandler.cpp
|  33| #include·"maths/Vector2D.h"
|    | [MAJOR] CPPCheckBear (syntaxError):
|    | Invalid number of character ({) when these macros are defined: 'MESSAGES_SKIP_STRUCTS'.

source/tools/atlas/GameInterface/Handlers/CinemaHandler.cpp
|  33| #include·"maths/Vector2D.h"
|    | [MAJOR] CPPCheckBear (syntaxError):
|    | Invalid number of character ({) when these macros are defined: '_MSC_VER'.
Executing section JS...

Link to build: https://jenkins.wildfiregames.com/job/differential/452/display/redirect

Did you test without PCH?

I'm not falling for that one again, always compiling --without-pch.

(The MapReader MapWriter files still don't contain all includes they should have, but I stopped looking)

Requested changes to bump the year, really?
Also not mentioning the controversy in a patch doesn't make it an uncontroversial patch. As far as I remember there are two different kinds of policies - one where includes are left out if they are already loaded in the TU elsewhere, and one where each file contains all includes (or forward decls) that it actually uses (regardless whether they were loaded in a different TU before).
One can spend many hours going through the includes, this patch is definitely incomplete, I just stopped aborted working on it after some time because who cares about includes if it compiles.
I suppose this will also cost an hour to go through every line again before the committer reviews it against just before committing. I guess the reviews are only here to show that one person didn't object rather than to verify patches.

Sounds like a neat system we got over here.

See also D1333 for precompiled include updates, and removals in that file.
Cleaning includes makes build faster.

If it can save me 20 mins when building the game over and over to make mac bundles, just do it :P

Requested changes to bump the year, really?

It takes it out of the review queue which I thought was WAD. I don't think we have actual guidelines for when to request changes, but maybe we should have.

Also not mentioning the controversy in a patch doesn't make it an uncontroversial patch. As far as I remember there are two different kinds of policies - one where includes are left out if they are already loaded in the TU elsewhere, and one where each file contains all includes (or forward decls) that it actually uses (regardless whether they were loaded in a different TU before).

This isn't a controversy, it's just bike shedding. AFAIK we've never decided on either of those two policies, and the second one seems to me like it would be a huge pain to maintain for literally no benefit since as you said it compiles anyways (and doesn't even change compile times, for that matter).

One can spend many hours going through the includes, this patch is definitely incomplete, I just stopped aborted working on it after some time because who cares about includes if it compiles.
I suppose this will also cost an hour to go through every line again before the committer reviews it against just before committing. I guess the reviews are only here to show that one person didn't object rather than to verify patches.

One can spend many hours going through the includes or one can commit this which remains a step forward, even if not completely correct.

We've already spent a year doing GDPR work, I'm not going to wait another year for all includes to be reviewed.