-
-
Notifications
You must be signed in to change notification settings - Fork 276
add ExplicitImports to CI check #4326
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
base: master
Are you sure you want to change the base?
Conversation
7916abb
to
b84e4d7
Compare
@ericphanson I'm struggling to get the recommended script to work because it expects ExplicitImports to be a dep of the main Pkg Project.toml and I don't think exposes a way to specify a different Project.toml and retain the pwd that files should be evaluated from? |
In the linting workflow, we explicitly add ExplicitImports and JuliaFormatter (https://github.com/JuliaBesties/BestieTemplate.jl/blob/main/.github/workflows/Lint.yml#L32). The |
54a9d5e
to
f7dffce
Compare
I changed this to just run on CI. @ericphanson, are these bugs in ExplicitImports: using UUIDs: UUIDs # used at /Users/kc/JuliaPkgs/Pkg.jl/src/REPLMode/REPLMode.jl:8:7
using Dates: @dateformat_str # used at /Users/kc/JuliaPkgs/Pkg.jl/src/Registry/Registry.jl:668:57 The first one seems wrong, at that code location the source code is using UUIDs: UUID The second one has an explicit import for that symbol: using Dates: @dateformat_str, Dates, DateTime, Millisecond, Second, now |
14befd3
to
4f1417e
Compare
probably... IIRC there is an issue where if I removed these it would fail to recognize that |
I don't really understand this:
Artifacts is a dependency... Maybe it collides with Pkg.Artifacts somehow. |
4f1417e
to
2d51a88
Compare
Or PkgArtifacts? #4230 |
what is going on here... why cant it load any packages |
for depot in Pkg.depots() | ||
for depot in Pkg.Base.DEPOT_PATH |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems odd?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I removed depots()
and replaced it with Base.DEPOT_PATH. This is weird (should just be Base.DEPOT_PATH
) but I don't think it should change anything..
Had this in #4288