All of lore.kernel.org
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
Cc: Han-Wen Nienhuys <hanwen@google.com>,
	Han-Wen Nienhuys via GitGitGadget <gitgitgadget@gmail.com>,
	git@vger.kernel.org, Han-Wen Nienhuys <hanwenn@gmail.com>
Subject: Re: [PATCH v2] refs.h: make all flags arguments unsigned
Date: Thu, 03 Feb 2022 10:27:43 -0800	[thread overview]
Message-ID: <xmqqo83nvm8w.fsf@gitster.g> (raw)
In-Reply-To: <220203.8635kz6d2o.gmgdl@evledraar.gmail.com> (=?utf-8?B?IsOG?= =?utf-8?B?dmFyIEFybmZqw7Zyw7A=?= Bjarmason"'s message of "Thu, 03 Feb 2022 18:53:55 +0100")

Æ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.




  parent reply	other threads:[~2022-02-03 18:27 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-01-31 20:15 [PATCH] refs.h: make all flags arguments unsigned Han-Wen Nienhuys via GitGitGadget
2022-02-01  2:02 ` Junio C Hamano
2022-02-01 11:47   ` Han-Wen Nienhuys
2022-02-01 18:41     ` Junio C Hamano
2022-02-01 12:46 ` [PATCH v2] " Han-Wen Nienhuys via GitGitGadget
2022-02-01 20:20   ` Ævar Arnfjörð Bjarmason
2022-02-01 23:03     ` Junio C Hamano
2022-02-03 14:29       ` Han-Wen Nienhuys
2022-02-03 17:53         ` Ævar Arnfjörð Bjarmason
2022-02-03 18:16           ` Han-Wen Nienhuys
2022-02-03 21:20             ` Ævar Arnfjörð Bjarmason
2022-02-03 18:27           ` Junio C Hamano [this message]
2022-02-03 18:33             ` Han-Wen Nienhuys
2022-02-03 19:15               ` Junio C Hamano
2022-02-03 14:26   ` [PATCH v3 0/2] " Han-Wen Nienhuys via GitGitGadget
2022-02-03 14:26     ` [PATCH v3 1/2] " Han-Wen Nienhuys via GitGitGadget
2022-02-03 14:26     ` [PATCH v3 2/2] Uniformize flag argument naming to `flags` or `unused_flags` Han-Wen Nienhuys via GitGitGadget

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqqo83nvm8w.fsf@gitster.g \
    --to=gitster@pobox.com \
    --cc=avarab@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitgitgadget@gmail.com \
    --cc=hanwen@google.com \
    --cc=hanwenn@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.