On 10/18/21 2:57 PM, David Kahurani wrote:
> The following patches introduce the FIXME_ONCE macro
> which suppresses repeated FIXMEs into WARNings. The current
> FIXME macro tends to be insufficient in cases where
> a developer wishes to suppresses fixmes other than the first.
> It is based on the vkd3d version.
>
> v2: Fix programming mistakes, fixed a test and some formating
>
> David Kahurani (21):
Hi David,
I understand this can look better at first but there's additional things
to take into consideration, and especially what a change like this implies.
First of all, this is changing a header used in absolutely all Wine
source code. This means that such change will require other developers
to fully rebuild Wine, and things like ccache won't help at all.
This better has to be really well justified, and maybe subject to prior
discussion and general agreement unless there's no doubt it's needed
(like adding Windows declarations in public header files).
I don't think this is the case here: the change introduces a new macro
that will only save one line of code everywhere it's used.
Second, every patch sent on this mailing list is going to be built and
eventually tested by an automated test bot. Like for the first point, as
this series is modifying a global header used everywhere else, the bot
is going to have to rebuild everything from scratch.
This will take a certain amount of time and resources... but moreover,
it's going to have to do it for every single patch of the series, and
that is going to take ages. As you may have noticed, your previous
attempt took around 18h to build and test all the patches, and during
that period of time the test bot is going to be fairly unavailable for
any other developer or patch (there's some priorities involved but still).
In conclusion, I think I can say this is very unlikely to be accepted,
because of the limited usefulness I described in the first part, but
even if it was you should probably start sending much much smaller
batches if you change global headers.
I have considered all that you described. This change was suggested here on the list a while back and I took it up. The problem with the patchset is that they go into different subsystems which forces me to split them up. They can obviously be bundled into one single patch or maybe just one patch demonstrating the use.
Cheers,
--
Rémi Bernon <rbernon@codeweavers.com>