LGTM

LGTM

LGTM.
This seems a proper GTest way, indeed.
Just for the record, the warning was reported by clang 5.0.1.

LGTM

LGTM

LGTM.
There is a cosmetic issue, but your change seems in line with the rest of the file, so, not sure if you would want to do this just for 2 ninja commands you are adding, or address it everywhere in this file as a separate patch.

LGTM

WithProperties should work just fine in this context.
It gets rendered for each build, but will always be the same, unless someone would overwrite it for a particular build.

Hello Jonas.
This approach does not support a build slave "jobs" property, making it hard to support multiple slaves for the same builder.
However, since this change is local for OpenMPBuilder, you may go ahead and commit it, if you are not up to a better implementation at the moment.

Let's keep only the changes relevant to this patch. Feel free to make a separate cosmetic NFC patch if you'd like.
Please feel free to commit, assuming you would roll that leading whitespace back.

Hi Jonas,

Thanks for splitting the original review item.

Hi Pavel,

When you force to build a revision 0, you would get a red build before the next one will do a clean checkout. Which is not always good.

This seems like 2 unrelated changes.
Could you split it to 2 different items, please?

Not sure the naming is good, though. Since this is a build property, it might be a good idea to be a bit more specific of what kind of mode is that, what do you think?
How about "checkout_mode" or "svn_mode"?

LGTM

LGTM, assuming you will remove the unrelated cosmetic changes, and figure out what place you want to specify the number of jobs. Up to me, a slave property seems a better and more natural place.