Conversation
|
@cpu ping? I do not mean to hurry. I totally understand if you do not have time, just tell me :) |
|
Thanks for the ping and sorry for letting it sit without a reply! I'm a bit more over-subscribed than usual this week and probably won't have a chance to look at it until ~next. You don't have to wait for me of course if that's too long to wait. |
|
Quick question, is the code in this PR what landed in the 0.18 release? Thank you all |
|
Hi @cpu,
|
cpu
left a comment
There was a problem hiding this comment.
Thanks for the ping and apologies for letting this fall off my radar! Here's some initial feedback.
| [](https://github.com/rusticata/x509-parser/actions) | ||
| [](#rust-version-requirements) | ||
|
|
||
| <!-- cargo-rdme start --> |
There was a problem hiding this comment.
Unfortunately it seems like this commit is breaking many markdown links in the rendered preview (https://github.com/rusticata/x509-parser/blob/fe005007af027b0cbf24f8a8bd8fadac204df527/README.md)
I think the link targets aren't quite right.
There was a problem hiding this comment.
This is an annoying limitation of cargo rdme: not all intradoc links are supported, for ex [`visitor`] is not replaced.
I found a workaround by giving the full path [`X509Certificate`](crate::certificate::X509Certificate) but now cargo doc is complaining that the path is redundant
If I push a commit to use this workaround, this will add warnings when running cargo doc (and cannot fix all links).
The last solution is to strip the intradoc links when generating the README.
There was a problem hiding this comment.
About #230 , I cherry-picked the commit in this branch with a few changes to resolve the conflicts and change src/lib.rs as well as README.md.
There was a problem hiding this comment.
This is an annoying limitation of cargo rdme: not all intradoc links are supported, for ex [
visitor] is not replaced.
Bummer :-(
Would you be open to a more direct/dumber solution? We use a small script in rustls that slurps the header rustdoc from lib.rs and inserts it into the README at a fixed location. In CI we run the same script and then error if there's a diff to make sure we always update lib.rs and not the copy in README.md.
I don't think we've had any significant trouble/friction with this approach but it's not quite as polished as a dedicated tool like cargo rdme.
If I push a commit to use this workaround, this will add warnings when running cargo doc (and cannot fix all links).
I wonder if it's possible to silence/allow just that one warning about redundant paths. That seems preferable to allowing all warnings, or stripping links if you want to stick with cargo rdme.
Misc changes to prepare first beta release (which will be 0.19.0-beta.1):
timeandthiserrortimedepends ontime-corewhich unfortunately requires rust 1.81, which explains the next commits--preciseflag withcargo updatecargo rdme, add CI checks and update linkssrc/lib.rsand README.md in sync