-
Notifications
You must be signed in to change notification settings - Fork 480
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
Write inventory files #2424
Write inventory files #2424
Conversation
Just skip writing the inventory if the function `Documenter.HTMLWriter.write_inventory` exists, indicating that the installed version of Documenter can write inventories itself. For compatibility with JuliaDocs/Documenter.jl#2424
26baf37
to
5e4636a
Compare
cf04715
to
d59c529
Compare
d59c529
to
e8a8aae
Compare
I've marked this as "Ready for review" since it now has tests and documentation (no CHANGELOG yet). I agree with #2366 (comment), though: we should probably choose one format. I'd also lean towards the Sphinx The documentation is already written to only mention |
There's also the follow-up #2433 which adds the option for the There should be some way to specify a version, and ideally, it would be a solution so that when the new version of Documenter is released and starts to be used, existing projects end up with the correct That's why in #2433, I also added the fallback of searching for a |
Community call decision was to only write the Sphinx-format |
e8a8aae
to
bf4c18b
Compare
@mortenpi Okay, should be ready for merging or further comments. There was actually still a bug: #2433 is also ready. If you like #2433, you could just change the base branch for that PR to Edit: I forgot to update the CHANGELOG. I'll do it tomorrow, or maybe we can decide first how we want to handle #2433 so I can do one changelog for the combined changes. |
What's the fallback for I know you don't like this idea, but I still feel that the "right way" to do it right now is to patch the inventory file in the Using |
Also, for |
Yes, empty string and a warning to set a manual version as a parameter to
Indeed :-)
Alright, fair enough. Let me have a stab at that. I'll make an alternative to #2433. You're okay with the new |
I actually have very strong opinions on that, but you're right, most people aren't very strict or consistent about that.
It's probably not that critical what exactly the |
Actually... I was kinda hoping we wouldn't need it 😅 But I would be okay with One alternative possibility: we can have a public API function that you can use to patch the inventory file, which On |
I’m fine with |
Just to keep the discussion up-to-date here, we now have three competing PRs for completing this (regarding what is the best way to set the inventory
|
Removes the code for writing `inventory.toml.gz`.
e3f602b
to
45696e1
Compare
* Obtain inventory version from Project.toml * Overwrite inventory version in `deploydocs` This unconditionally overwrites an existing inventory `version` with the version determined by `deploydocs`. This only happens when `deploydocs` actually knows the version. For `dev`-deployments, an existing `version` is left unchanged. Under normal circumstances, the existing `version` will already be the correct one, so this should do nothing. If the `deploydocs` actually has to *change* the version, that would indicate that something is set up incorrectly, and thus a warning will be printed. * Restrict inventory version from `Project.toml` to standard setups In order to prevent an auto-detected inventory version being incorrect for unusual setups (e.g., monorepo), we only consider a `docs/../Project.toml` file when looking up a version. Everything else will leave the version blank in `makedocs`, respectively require explicitly setting `inventory_version` manually
This has been updated by squash-merging #2449 as the solution for setting the I've also added a CHANGELOG entry. |
@mortenpi This should be ready now for a final review (if there's any more bikeschedding) and merging. There is one issue we might still have to think about: Since we've settled on auto-detecting the version only for "standard setups", that no longer includes the On the other hand, they might also pin a specific Documenter version, and in that case we'll have to make a PR to Julia to update that to the new release anyway, which then might as well include setting I've rebased everything onto the current I've assumed that this will make it into a After we make the release, it might make sense to make some kind of announcement on Discourse about the new inventory feature. I'll also want to make a separate announcement of the |
Thanks to @vchuravy's help on Slack, I was able to determine that the Julia documentation builds pretty much just by running There is a custom However, there is also a I would still include adding an Bottom line: Julia's peculiarities are not a blocker for merging this PR, but we will have to follow up in order to ensure that future deployments of Julia's documentation have an inventory. |
Actually, we can tag it right away, but maybe hold off a few days with any kind of Discourse announcement. That might even give us a chance to
and for a handful of other packages to make a release/build with the newest Documenter before we go public; so it's not quite "you can link to any other project, but there's basically nothing yet that actually has an inventory" (despite the handful of inventories I've collected in the wiki) |
5c10fe0
to
35b3a95
Compare
It turns out the inventory-writing-code is more or less compatible all the way back to |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for the delay, but LGTM!
@mortenpi Can I merge this now (manually)? Ideally, before we merge too many other PRs, since otherwise I'll have to go through the rebase and conflict resolution again. |
Yeah, let's merge! |
Ah, branch protection! I'd still kinda like to preserve some of those commit messages with the references to the discussion, so not sure what to do. Maybe temporarily remove the protection, or maybe you can push with higher permissions than me. (On the other hand, the references are also in the PR comments, so I guess the usual squash would be ok, too. It's up to you.) |
Let's just squash merge this. The PRs offer a better context for the decisions here I think. |
With the newest Documenter release, the Julia documentation will automatically have an `objects.inv` file. This file allows any other project using Documenter with the [`DocumenterInterLinks`](https://github.com/JuliaDocs/DocumenterInterLinks.jl) plugin (or any project using [Sphinx](https://www.sphinx-doc.org/en/master/)) to link directly into the Julia documentation, e.g., with ```[`Base.sort!`](@extref Julia)```. See also JuliaDocs/Documenter.jl#2424 (comment) and the following comments
With the newest Documenter release, the Julia documentation will automatically have an `objects.inv` file. This file allows any other project using Documenter with the [`DocumenterInterLinks`](https://github.com/JuliaDocs/DocumenterInterLinks.jl) plugin (or any project using [Sphinx](https://www.sphinx-doc.org/en/master/)) to link directly into the Julia documentation, e.g., with ```[`Base.sort!`](@extref Julia)```. See also JuliaDocs/Documenter.jl#2424 (comment) and the following comments (cherry picked from commit 0311aa4)
With the newest Documenter release, the Julia documentation will automatically have an `objects.inv` file. This file allows any other project using Documenter with the [`DocumenterInterLinks`](https://github.com/JuliaDocs/DocumenterInterLinks.jl) plugin (or any project using [Sphinx](https://www.sphinx-doc.org/en/master/)) to link directly into the Julia documentation, e.g., with ```[`Base.sort!`](@extref Julia)```. See also JuliaDocs/Documenter.jl#2424 (comment) and the following comments
This automatically writes inventory files
objects.inv
andto theinventory.toml.gz
build
folder, which should get deployed together with the rest of the documentation. The presence of these files allows theDocumenterInterLinks
plugin to link into the documentation from any other project with an@extref
link.The PR adds a single new dependency:
CodecZlib
. This is necessary because inventory files should definitely be compressed (they potentially get pulled a lot, every time someone builds a documentation that has an@extref
into the project). TheCodecZlib
is maintained by the JuliaIO org and should be quite stable.Also see the discussion in #2366 (comment)
Closes #2366