-
-
Notifications
You must be signed in to change notification settings - Fork 283
fix unused imports reported by ExplicitImports.jl #4289
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
Conversation
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.
Pull Request Overview
This PR cleans up unused and overly broad imports flagged by ExplicitImports.jl by switching to targeted using/import statements, unqualifying function calls where appropriate, and adds a CI job to enforce future import hygiene.
- Replace fully-qualified calls (e.g.,
LibGit2.close,Pkg.generate) with unqualified functions and tighten imports to bring only needed symbols into scope. - Refactor dozens of modules to use
using Module: symbolorimport Module: symbolrather than importing entire modules. - Add a new GitHub Actions job under
.github/workflows/check.ymlthat runs ExplicitImports.jl checks on the codebase.
Reviewed Changes
Copilot reviewed 19 out of 19 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| test/repl.jl | Replaced LibGit2.close(...) with close(...). |
| test/pkg.jl | Similar unqualification of LibGit2.close(...). |
| src/precompile.jl | Converted Pkg.-prefixed calls to unqualified names and tightened imports. |
| src/fuzzysorting.jl | Dropped FuzzySorting. prefixes on fuzzyscore and levenshtein. |
| src/Types.jl | Switched to named using/import for external modules and removed unused imports. |
| src/Resolve/Resolve.jl | Refined imports for Printf, Random, and UUIDs. |
| src/Registry/Registry.jl | Tightened imports, changed default depots to Base.DEPOT_PATH, and unqualified close. |
| src/REPLMode/REPLMode.jl | Scoped imports for Dates, Markdown, and UUIDs. |
| src/PlatformEngines.jl | Scoped imports for SHA, Downloads, Tar, and p7zip_jll. |
| src/Pkg.jl | Scoped Dates imports, updated depots1 default, and unqualified internal calls. |
| src/Operations.jl | Cleaned up imports and unqualified calls to is_tracking_repo, close, etc. |
| src/MiniProgressBars.jl | Scoped imports of @sprintf from Printf. |
| src/GitTools.jl | Scoped SHA and Printf, switched ensure_clone to local clone. |
| src/Artifacts.jl | Explicitly imported artifact API symbols and sha256. |
| src/Apps/Apps.jl | Scoped TOML and UUIDs imports and removed unused Pkg.Types imports. |
| src/API.jl | Scoped imports, replaced depots() with Base.DEPOT_PATH, and unqualified calls. |
| ext/REPLExt/precompile.jl | Unqualified precompile calls and tightened import clauses. |
| ext/REPLExt/REPLExt.jl | Scoped using/import for Markdown, Dates, and UUIDs. |
| .github/workflows/check.yml | Added ExplicitImports.jl CI job to catch future unused or implicit imports. |
Comments suppressed due to low confidence (4)
test/repl.jl:31
- The call to
closeis now unqualified butLibGit2.closewas removed. You should bringcloseinto scope (e.g.using LibGit2: close) or revert toLibGit2.closeto avoid an undefined function error.
close((LibGit2.init(".")))
test/pkg.jl:617
- Similar to the other test,
closeis unqualified but not imported. Addusing LibGit2: closeor qualify asLibGit2.closeto ensure the function is available.
close(LibGit2.clone(TEST_PKG.url, "Example.jl"))
src/Types.jl:18
- You export
SHA1but removed its import. Addimport Base: SHA1so that theSHA1type is defined and can be exported.
using SHA: SHA, sha1
src/Registry/Registry.jl:269
- You replaced
LibGit2.close(repo)withclose(repo)but did not importclose. Considerusing LibGit2: closeor qualifying asLibGit2.close.
close(repo)
use ExplicitImports.jl to remove implicit imports from `using` etc
3f183e6 to
c08a176
Compare
not sure what that means, I also don't get it locally... |
|
@ericphanson any idea w.r.t the comment above? |
|
it is supposed to skip flagging usages of names from Base/Core by default, looks like that isn't happening here for some reason. If it doesn't repro locally that adds credence to it being a bug that was fixed since the old version used in the CI script |
Co-authored-by: Eric Hanson <[email protected]>
|
Looks good now |
There are more
importswe can move tousing :but this cleans up quite a bit for now. Also adds some CI to catch future things.