Conversation
danielhenrymantilla
force-pushed
the
initial-release
branch
from
bdf1b2f to
7fbdcb0
Compare
danielhenrymantilla
force-pushed
the
initial-release
branch
from
7fbdcb0 to
49852e1
Compare
| # `docs-rs` is very similar to `better-docs`, but for it being allowed to enable | ||
| # other features as well, often to make sure optional/non-`default` features | ||
| # make it to the rendered docs (using `--all-features` instead is not that great). | ||
| docs-rs = [ | ||
| "better-docs", | ||
| ] |
There was a problem hiding this comment.
Does it not make more sense for this to be a --cfg flag?
There was a problem hiding this comment.
I've seen this pattern quite a bit, true, but I somehow find the feature to be more convenient, provided that it is properly document as unstable (I'm not that fond of fiddling with env vars / rustflags unless strictly necessary). But I have no strong opinions on this either way 🙂
There was a problem hiding this comment.
I'm used to cargo build --all-features working on Tokio with a stable compiler, which is why we use a --cfg. But I guess you also have other unstable features here.
There was a problem hiding this comment.
Yeah, currently the default-features plays the role of the --all-features of finer-featured crates such as tokio; I wouldn't expect us going that far with this crate, but if we did, I suspect we'd end up with an all-stable-features Cargo feature for this (which is partially orthogonal to the specific case of docs.rs tweaks, since these are not expected to be used by users, so a --cfg can make a lot of sense there).
If you feel like making the change for these docs.rs knobs anyways feel free to do so or suggested it over #2
There was a problem hiding this comment.
It doesn't seem like better-docs or docs-rs are used at all? Maybe I'm missing something.
There was a problem hiding this comment.
Ah true 😄 This is from a template I use, which lets me opt into extra facilities. For instance, it has recently occurred to me that we could consider feature-gating the MaybeDangling wrapper (since I expect to be rarely needed to have something with inherent drop glue and yet allowed to dangle), and in that case the idea would be to use better-docs to enable doc(cfg()) to illustrate that
Sure, I do not mind. |
| impl<T> ::core::ops::Deref for MaybeDangling<T> { | ||
| type Target = T; | ||
|
|
||
| #[inline] | ||
| fn deref(self: &Self) -> &T { | ||
| let Self { value, .. } = self; | ||
| value | ||
| } | ||
| } |
There was a problem hiding this comment.
Why are they nested like this?
There was a problem hiding this comment.
It's an opinionated aspect of mine 😅; when there is a DerefMut I don't find that the Deref impl itself provides that much valuable info, so I prefer to reduce its visibility when skimming through all the main impls. Again, if this is to be co-owned I don't want to impose my views if the feeling is not shared, so we can normalize this over #2 as well
Happy to help out, but I'm bad at the “reviewing things on a consistent schedule“ aspect of “maintenance”. Do as you see fit. I took a look through the code and I might come up with a few polishing PRs. |
There was a problem hiding this comment.
I wonder why Cargo.lock is in source control? I always thought libraries don't do that
There was a problem hiding this comment.
There was a problem hiding this comment.
Thanks for sharing the link, the PR is quite informative. I got curious about why the lockfile was included while trying to understand why CI is testing against the locked and latest versions of the (hypothetical) dependencies. This conservative approach to CI testing both deterministically and non-deterministically makes sense to me after reading the PR. Never really thought about dependencies breaking pipelines.
There was a problem hiding this comment.
I've also written my own take on the topic: https://gist.github.com/danielhenrymantilla/9b59de4db8e5f2467ed008b3c450527b#about-tracking-cargolock
4 participants
cc @Darksonn @kpreid @jofas: do you want to coöwn and co-maintain this crate?