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

update patch documentation #439

Merged
merged 3 commits into from
Feb 27, 2021
Merged

update patch documentation #439

merged 3 commits into from
Feb 27, 2021

Conversation

clux
Copy link
Member

@clux clux commented Feb 27, 2021

  • kube::api::Patch now documents how to create a patch in the two normal ways + notes merge strat differences
  • kube-derive now documents how to customize merge strats + schemas in general
  • cross-linking

@clux clux linked an issue Feb 27, 2021 that may be closed by this pull request
@clux clux changed the title update patch documentation - closes #142 update patch documentation Feb 27, 2021
@clux clux marked this pull request as ready for review February 27, 2021 13:41
@clux clux requested a review from kazk February 27, 2021 13:41
Copy link
Member

@kazk kazk left a comment

Choose a reason for hiding this comment

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

Looks good to me. Left some comments.

kube-derive/src/lib.rs Outdated Show resolved Hide resolved
Makefile Outdated
@@ -12,7 +12,7 @@ fmt:
cargo +nightly fmt

doc:
cargo +nightly doc --lib
cargo +nightly doc --lib --features=derive,ws,oauth,jsonpatch
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
cargo +nightly doc --lib --features=derive,ws,oauth,jsonpatch
RUSTDOCFLAGS="--cfg docsrs" cargo +nightly doc --lib --features=derive,ws,oauth,jsonpatch

To see feature tags, if I remember correctly. Appending -- --cfg docsrs might work as well.

Copy link
Member Author

Choose a reason for hiding this comment

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

That would be great, but for some reason, neither of those work as expected. At least not locally. I need to explicitly pass the features to get them to build with cargo +nightly doc --lib, and -- --cfg also doesn't work.

Copy link
Member Author

Choose a reason for hiding this comment

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

I think the -- --cfg flag is for cargo rustdoc and that only works on a single manifest, whereas cargo doc works on a workspace.

cargo rustdoc never seems to get the link between kube and kube_derive::CustomResource (which is re-exported under a feature).

Copy link
Member Author

@clux clux Feb 27, 2021

Choose a reason for hiding this comment

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

Ok cargo +nightly doc --lib --workspace picks up on all crates in the workspace correctly, but it still won't grab the default feature list from kube/Cargo.toml's package.metadata.docs.rs.

See rust-lang/cargo#8944

Copy link
Member

Choose a reason for hiding this comment

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

Ah, I was running the command in kube/.

From the root, the following worked for me:

RUSTDOCFLAGS="--cfg docsrs" cargo +nightly doc --lib --workspace --features=derive,ws,oauth,jsonpatch --open

Documented all crates and got feature tags in kube. --open opens the documentation in the browser.

Copy link
Member Author

@clux clux Feb 28, 2021

Choose a reason for hiding this comment

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

Ah, --open is nice!

Was hoping that we could avoid duplicating the docsrs feature list, but whatever, it's only for local dev :-)

kube-derive/src/lib.rs Show resolved Hide resolved
@clux clux merged commit 00e596b into master Feb 27, 2021
@clux clux deleted the patch-docs branch February 27, 2021 23:20
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.

Should Api handle serialization too?
2 participants