On Tue, Aug 31 2021, Carlo Arenas wrote: > On Mon, Aug 30, 2021 at 4:40 AM Ævar Arnfjörð Bjarmason > <avarab@xxxxxxxxx> wrote: >> On Mon, Aug 09 2021, Phillip Wood wrote: >> > On 09/08/2021 02:38, Carlo Marcelo Arenas Belón wrote: >> >> similar to the recently added sparse task, it is nice to know as early >> >> as possible. >> >> add a dockerized build using fedora (that usually has the latest >> >> gcc) >> >> to be ahead of the curve and avoid older ISO C issues at the same time. >> > >> > If we want to be able to compile with -Wpedantic then it might be >> > better just to turn it on unconditionally in config.mak.dev. Then >> > developers will see any errors before they push and the ci builds will >> > all use it rather than having to run an extra job. I had a quick scan >> > of the mail archive threads starting at [1,2] and it's not clear to me >> > why -Wpedaintic was added as an optional extra. >> >> This is from wetware memory, so maybe it's wrong: But I recall that with >> DEVOPTS=pedantic we used to have a giant wall of warnings not too long >> ago (i.e. 1-3 years), and not just that referenced >> USE_PARENS_AROUND_GETTEXT_N issue. > > when gcc (and clang) moved to target C99 by default (after version 5) > then that wall of errors went away. Indeed git can build cleanly in a > strict C99 compiler and until reftable was able to build even with gcc > 2.95.3 > > the nostalgic can get it back with `CC=gcc -std=gnu89`, and indeed I > was considering this might be a good alternative to the defunct > gcc-4.8 job, where the weather balloons breaking with strict C89 > compatibility could be explicitly coded. > >> So if we turn pedantic on in DEVOPTS by default, wouldn't it make sense >> to at least have a CI job where we test that we compile with >> USE_PARENS_AROUND_GETTEXT_N (which at that point would not be the default >> anymore). > > agree, and indeed was thinking it might be worth combining this job > with the SANITIZE one for efficiency. On the other hand maybe we should just remove USE_PARENS_AROUND_GETTEXT_N entirely, i.e. always use the parens. That facility seems to have been added in response to a one-off mistake in 9c9b4f2f8b7 (standardize usage info string format, 2015-01-13). See https://lore.kernel.org/git/ecb18f9d6ac56da0a61c3b98f8f2236@74d39fa044aa309eaea14b9f57fe79c/. That later landed as 290c8e7a3fe (gettext.h: add parentheses around N_ expansion if supported, 2015-01-11). It doesn't seem worth the effort to forever maintain this special case and use CI resources etc. to catch what was effectively a one-off typo.
