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.
