Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5afbcf6136
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| let args: Vec<OsString> = env::args_os().skip(1).collect(); | ||
| let mut command = Command::new(&cargo_dylint); | ||
| command.arg("dylint"); | ||
| command.arg("--lib-path").arg(&library_path); | ||
| if !has_library_selection(&args) { |
There was a problem hiding this comment.
If someone invokes the released argument-comment-lint the same way CI invokes tools/argument-comment-lint/run.sh (for example from the repo root with -p codex-core), this wrapper just forwards the current working directory to cargo-dylint. Because the workspace manifest lives under codex-rs/Cargo.toml, cargo-dylint will fail to find a manifest unless the caller also remembers the --manifest-path/--workspace/--no-deps defaults that run.sh injects today. That makes the new release asset non-runnable for the repo's normal entrypoint instead of being a drop-in packaged replacement.
Useful? React with 👍 / 👎.
| - runner: ubuntu-24.04 | ||
| target: x86_64-unknown-linux-gnu | ||
| archive_name: argument-comment-lint-x86_64-unknown-linux-gnu.tar.gz | ||
| lib_name: libargument_comment_lint@nightly-2025-09-18-x86_64-unknown-linux-gnu.so | ||
| runner_binary: argument-comment-lint |
There was a problem hiding this comment.
These release assets are built as *-unknown-linux-gnu on Ubuntu 24.04 runners, and the archive also bundles a host-built cargo-dylint. On older-but-still-supported Linux environments (for example Ubuntu 20.04 / Debian 10 from docs/install.md), that package can fail before linting with a GLIBC version error. The main release workflow uses musl targets for shipped Linux binaries, so this new DotSlash package would be the one released tool that does not run across the repo's supported Linux baseline.
Useful? React with 👍 / 👎.
2 participants
Why
To date, the argument-comment linter introduced in #14651 had to be built from source to run, which can be a bit slow (both for local dev and when it is run in CI). Because of the potential slowness, I did not wire it up to run as part of
just clippyor anything like that. As a result, I have seen a number of occasions where folks put up PRs that violate the lint, see it fail in CI, and then have to put up their PR again.The goal of this PR is to pre-build a runnable version of the linter and then make it available via a DotSlash file. Once it is available, I will update
just clippyand other touchpoints to make it a natural part of the dev cycle so lint violations should get flagged before putting up a PR for review.To get things started, we will build the DotSlash file as part of an alpha release. Though I don't expect the linter to change often, so I'll probably change this to only build as part of mainline releases once we have a working DotSlash file. (Ultimately, we should probably move the linter into its own repo so it can have its own release cycle.)
What Changed
rust-release-argument-comment-lint.ymlworkflow that builds host-specific archives for macOS arm64, Linux arm64/x64, and Windows x64rust-release.ymlto publish theargument-comment-lintDotSlash manifest on all releases for now, including alpha tagsThe Unix archive layout is:
On Windows the same layout is published as a
.zip, with.exeand.dllfilenames instead.DotSlash resolves the package entrypoint to
argument-comment-lint/bin/argument-comment-lint. That runner finds the sibling bundledcargo-dylintbinary plus the single packaged Dylint library underlib/, then invokescargo-dylint dylint --lib-path <that-library>with the repo's default lint settings.