Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

What should our explicit import policy be? #2339

Open
MatthewDaggitt opened this issue Apr 1, 2024 · 10 comments
Open

What should our explicit import policy be? #2339

MatthewDaggitt opened this issue Apr 1, 2024 · 10 comments

Comments

@MatthewDaggitt
Copy link
Contributor

As discussed in #2334 we don't currently have a policy about when imports should use an explicit using statement and when it's fine to open the whole module. We should add something to the style guide.

I have no particularly strong feelings on the matter.....

@JacquesCarette
Copy link
Contributor

We certainly should allow explicit imports, even if they are long, and not remove them once they've been put into place.

To me, there should be some 'core' of the library where explicit imports are required, and an 'upper layer' where they are merely encouraged.

@gallais
Copy link
Member

gallais commented Apr 3, 2024

I would argue in favour of mandatory explicit import lists if they
mention, say, max half a dozen identifiers. Throwing these in as
we touch modules should hopefully help us detect spurious
dependencies (until we get better tooling to do it automatically).

@JacquesCarette
Copy link
Contributor

I'd be happy with mandatory for <= 6. I'd be unhappy to have explicit imports deleted just because they're long.

@jamesmckinna
Copy link
Contributor

I'd be happy with mandatory for <= 6. I'd be unhappy to have explicit imports deleted just because they're long.

NB some recent refactorings do indeed delete 'long' using directives, but usually only to localise them further by removing more names, or to distribute the names they do import over lower-level modules they import from... so I don't think there's unnecessary 'widening' of import interfaces going on here.

@jamesmckinna
Copy link
Contributor

jamesmckinna commented Feb 27, 2025

Re: @jmougeot 's recent long suite of PRs, where using directives are deployed to localise everything on import.

What do we do about the limit cases where eg X.Bundles requires each and every definition of the corresponding X.Structures, given our current hierarchy design principles. Should we mention them explicitly, or not? I would think not (too much knock-on viscosity/cognitive load), but interested to poll @gallais (arguing for the 'defensive' approach to API imports) and @JacquesCarette (arguing for explicitness everywhere for 'core' stdlib modules... see #2573 for an 'interesting' merge conflict resolution which arises from this) for their opinion...

@jamesmckinna
Copy link
Contributor

Separately, and belatedly, I've just noticed that, again in Jacques' long series of PRs, the following pattern of qualified import sometimes occurs:

import Foo as Foo using (foo)

and I'm really not sure whether this either makes sense, or is a good idea? (It does, however, typecheck, with foo used in qualified scope as Foo.foo, but then... why mention it in the first place?)

@JacquesCarette
Copy link
Contributor

JacquesCarette commented Feb 28, 2025

I've commented elsewhere on the Bundles vs Structures: at the use site, there is still cognitive load of having to remember everything that is in Structures. One could definitely make an exception for that one case since the point is to cover everything. I'm also of the 'defensive' school; when the list is short enough, I think it's a good idea to be explicit.

For the second case (qualified imports), I'm less sure. I still rather like the documentation aspect, but I would not fight very hard if the opposite choice was made.

@jamesmckinna
Copy link
Contributor

For the second case (qualified imports), I'm less sure. I still rather like the documentation aspect, but I would fight very hard if the opposite choice was made.

"would fight" or wouldn't fight?

@JacquesCarette
Copy link
Contributor

Oops: comment edited. 'not' was missing. Thanks for the gentle prod!

@jamesmckinna
Copy link
Contributor

Given the avalanche of related PRs that @JacquesCarette and @jmougeot have undertaken as related to this issue, might I suggest you both also raise a PR codifying the style-guide precepts which have emerged as a consequence? (And such a thing seems like a capstone PR to undertake after they have all been merged?)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants