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

feat(build): Make cleanup-markdown feature additive #1952

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

tottoto
Copy link
Collaborator

@tottoto tottoto commented Sep 21, 2024

Makes cleanup-markdown feature additive.

@@ -25,7 +25,7 @@ syn = "2.0"
[features]
default = ["transport", "prost"]
prost = ["prost-build", "dep:prost-types"]
cleanup-markdown = ["prost", "prost-build/cleanup-markdown"]
cleanup-markdown = ["prost-build?/cleanup-markdown"]
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why is the previous setup non-additive?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The cleanup-markdown feature does not assume prost in terms of meaning, so I used the term additive here in the sense that it will remove the behavior that implicitly enables prost and make it possible to be compatible with other codecs in the future. In fact, it is additive at present, so there may be a more appropriate representation.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Should there be some added documentation to make it clearer that cleanup-markdown will do nothing unless prost is also enabled?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Updated the document.

@tottoto tottoto force-pushed the make-cleanup-markdown-feature-additive branch from 3f919f0 to 93d74f4 Compare September 22, 2024 10:05
@@ -4,7 +4,8 @@
//! # Feature flags
//!
//! - `cleanup-markdown`: Enables cleaning up documentation from the generated code. Useful
//! when documentation of the generated code fails `cargo test --doc` for example.
//! when documentation of the generated code fails `cargo test --doc` for example. `prost`
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'd suggest adding "The" here: "The prost feature [..]".

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Applied the suggestion.

@tottoto tottoto force-pushed the make-cleanup-markdown-feature-additive branch from 93d74f4 to 5a53262 Compare September 22, 2024 10:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants