Skip to content

remove the prodash feature#2473

Closed
Alexander Kjäll (alexanderkjall) wants to merge 1 commit intoGitoxideLabs:mainfrom
alexanderkjall:remove-prodash-feature
Closed

remove the prodash feature#2473
Alexander Kjäll (alexanderkjall) wants to merge 1 commit intoGitoxideLabs:mainfrom
alexanderkjall:remove-prodash-feature

Conversation

@alexanderkjall
Copy link
Copy Markdown
Contributor

As that feature is just from an optional dependency, I think a consumer of this crate should use the progress-tree feature?

from: https://salsa.debian.org/rust-team/debcargo-conf/-/blob/master/src/gix/debian/patches/reduce-features.patch?ref_type=heads#L26

author: Fabian-Gruenbichler

As that feature is just from an optional dependency, I think a consumer of this crate should use the progress-tree feature
Copy link
Copy Markdown
Contributor

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 90cc5be06b

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "Codex (@codex) review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "Codex (@codex) address that feedback".

Comment thread gix/Cargo.toml
## Re-export the progress tree root which allows to obtain progress from various functions which take `impl gix::Progress`.
## Applications which want to display progress will probably need this implementation.
progress-tree = ["prodash/progress-tree"]
progress-tree = ["dep:prodash", "prodash?/progress-tree"]
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Preserve the public prodash feature alias

If any downstream manifest still enables gix with features = ["prodash"] (or forwards gix/prodash transitively), this change makes Cargo fail during feature resolution: using dep:prodash removes the implicit public prodash feature entirely. I verified that the parent revision still exposed prodash => ["dep:prodash"], while the new manifest rejects that feature name, so this is a breaking change for existing consumers rather than just an internal cleanup.

Useful? React with 👍 / 👎.

@Skgland
Copy link
Copy Markdown

This was intentionally changed from dep:prodash to prodash in #1024 to fix compatibility with cargo-auditable.
The upstream issue in cargo-auditable is still open rust-secure-code/cargo-auditable#124.

@Byron
Copy link
Copy Markdown
Member

Thanks a lot for contributing Alexander Kjäll (@alexanderkjall) , and for chiming in Bennet Bleßmann (@Skgland).

It seems there is a conflict of interest here, and I'd rather keep it like it is than to reintroduce another downstream issue - there is seemingly no way to fix both until the cargo auditable fix is made. I now subscribed to the issue, so hopefully we will be able to reopen this PR soon.

Thanks for your understanding.

PS: If there is other options, please let me know in the comments.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants