From: Emily Shaffer <nasamuffin@google•com>
To: Git List <git@vger•kernel.org>
Cc: chooglen@google•com, newren@gmail•com, cscallsign@gmail•com
Subject: How to discourage introduction of new globals? (was: Video conference libification eng discussion - notes from today)
Date: Thu, 20 Apr 2023 10:08:30 -0700 [thread overview]
Message-ID: <ZEFxjmt9zV6V384b@google.com> (raw)
In-Reply-To: <CAJoAoZnoz9rsEEUGoG4BKMwW7r_Q-H2JcD_SVxuA_ykDZ33J8w@mail.gmail.com>
On Thu, Apr 20, 2023 at 10:01:45AM -0700, Emily Shaffer wrote:
> * How can we avoid introducing new globals?
> * Elijah: for example, diffcore added a new global for actually no reason
> * Getting rid of the\_repository is so daunting
> * Emily: should we introduce some CI rule around new globals in
> the diff? Update SubmittingPatches?
> * Glen: tests which enforce libification also make it difficult
> to introduce new globals in libraries. But in some places, like
> setting something across the entire process lifetime, we don't have a
> better pattern to replace it with, right?
> * Elijah: in the short term, we do have dozens of globals;
> should they go in one place (so we know where to get rid of them)?
> Should they go with the file that is using them the most, or
> something?
> * Emily: does putting all the globals in one place make it
> easier to tell whether a given path is using a certain global?
> * Cem: Putting it in one file makes it really easy to tell
> people they shouldn't introduce a new global! E.g. adding a comment at
> the top of `globals.h` like /\* are you sure you need to add a global
> here? \*/
> * Emily: can coccinelle help?
> * Glen: probably not, coccinelle runs over all files whether
> they're unchanged or not
> * Emily: does wrapping globals in a getter that traces help?
> * Elijah: but people will just bypass the getter, unless you
> restructure the code so the accessors are the only thing that are
> accessible
> * Cem: This is basically a global to singleton conversion -
> but there's no way to guarantee that new globals follow that pattern
> either
> * Emily: I'll spin this out into a separate thread after this meeting
Any thoughts from the rest of the project? To summarize goals:
- The path to library code stays about as hard as it is, doesn't get
harder by the addition of more globals.
- It's very easy to tell when globals are being used in order to make
cleaning them up less painful.
- The onus for avoiding new globals doesn't fall entirely on a handful
of reviewers, who may miss something newly added.
prev parent reply other threads:[~2023-04-20 17:08 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-04-20 17:01 Video conference libification eng discussion - notes from today Emily Shaffer
2023-04-20 17:08 ` Emily Shaffer [this message]
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=ZEFxjmt9zV6V384b@google.com \
--to=nasamuffin@google$(echo .)com \
--cc=chooglen@google$(echo .)com \
--cc=cscallsign@gmail$(echo .)com \
--cc=git@vger$(echo .)kernel.org \
--cc=newren@gmail$(echo .)com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox