RFR: 8313302: Fix formatting errors on Windows [v3]
Thomas Stuefe
stuefe at openjdk.org
Fri Jul 28 10:25:41 UTC 2023
On Fri, 28 Jul 2023 07:50:19 GMT, Julian Waters <jwaters at openjdk.org> wrote:
>> Fix several formatting errors on Windows
>
> Julian Waters has updated the pull request incrementally with one additional commit since the last revision:
>
> zPhysicalMemoryBacking_windows.cpp
> > Hi Julian,
> > I'm sorry, but I object to this change.
> > It is another heap of aesthetic changes, with very very few changes actually needed (see inline remarks). Contrary to the title, it also touches shared code, and there are (almost) no errors to fix. And the one arguably incorrect place, in code heap, is not a "formatting error on windows".
> > Please consider that your aesthetic taste is not shared by everyone, but changes like these cause a lot of work for others.
> > Cheers, Thomas
>
> Hi Thomas,
>
> Sigh, I was afraid of that. I've not been putting out good quality changes lately :( I will mention that these are not merely aesthetic changes however, but actually is an effort spawned out of JDK-8288293, since all of these are flagged as formatting errors. It may be the case that I'm putting a bit too much faith in gcc's format checking, but I do want to discuss the rest in the review comments since it is a bit easier
So the point of this change is to satisfy gcc on Windows? Accomodating a new build platform, making (and keeping!) it warning-free is a considerable effort. Even if you do it, it has a lot of side effects on others: reviewer churn, accidental bugs, makes backports more difficult...
For smallish things its okay, but if it keeps causing massive changes like this one we should discuss this first. In my eyes, this is similar to adding a new platform, for which the bar is very (it is technically less complex than a new platform, but OTOH it is also less isolated).
As a side note, I think I mentioned this before, but it would be very helpful if you would put more love into your JBS issue descriptions and - texts.
Cheers, Thomas
-------------
PR Comment: https://git.openjdk.org/jdk/pull/15063#issuecomment-1655447802
More information about the graal-dev
mailing list