Re: [PATCH v2] refs.h: make all flags arguments unsigned

From: Junio C Hamano <gitster@pobox.com>
Date: Feb 3, 2022, 18:27 UTC
Message-ID: <xmqqo83nvm8w.fsf@gitster.g>
In-Reply-To: <220203.8635kz6d2o.gmgdl@evledraar.gmail.com>

Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:

> Yes, enums or not, what I was also pointing out in
> https://lore.kernel.org/git/220201.86ilty9vq2.gmgdl@evledraar.gmail.com/
> is that changing just one logical set of flags at a time would make this
> much easier to review.

Another thing to consider is how to make this play better with other topics in flight. Basing a huge single patch on top of 'seen' is a way to ensure that the patch will never be useful. There won't be a good time when such a topic can graduate. The topic will also have a hard time keeping up with what new topics add while waiting for what happen to be in 'seen' today (some of which may even go away without graduating) all graduate.

Limiting the scope to small and more stable subset of flags that are in 'master' and does not conflict (e.g. no new bit defined to the set of flags, no existing bit gets removed, no new callers that use the bitset introduced) with other topics would incrementally improve the code base, and makes progress in the sense that it reduces the remaining work.

> It doesn't matter for the end result as long as we end up with "unsigned
> int" everywhere, but would with enums.

As it won't be an error to assign to what has converted to enum a value that is in int or unsigned that comes from the part of the code base that is not yet converted, so it may be OK either way.

Assigning between converted "unsigned" and unconverted "int" may add unnecessary warnings during transitory period, but I think the story would be the same for a flag bitset that is conveted into an "enum", which the compiler happens to make it "unsigned", which was originally a signed "int".

Thanks.