How to discourage introduction of new globals? (was: Video conference libification eng discussion - notes from today)
On Thu, Apr 20, 2023 at 10:01:45AM -0700, Emily Shaffer wrote:
Show 30 quoted lines
> * 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.
Emily Shaffer <nasamuffin@google.com>