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

Docs: Restructure docs folder #1438

Merged
merged 20 commits into from
Feb 20, 2024

Conversation

har7an
Copy link
Contributor

@har7an har7an commented Feb 6, 2024

Sort of a follow-up to #1315 which revealed a "mismatch" between what the README and docs/ folder immediately provide as info compared to what's actually available.

This restructures the docs/ folder and adds information from various sources to it:

  • The README,
  • The Wiki
  • Examples found in the Wiki

At the moment this duplicates a decent amount of information already available on the Wiki. Strictly speaking this was already the case before this MR, where the README and cross_toml each contained information from the Wiki. My motivation for moving these things into the docs/ folder is that:

  1. I'm much more used to finding information in a docs folder than to check for the Wiki. Most projects just don't use the Wiki
  2. I usually expect the documentation to be part of what I check out, and the Wiki is not a part of this repo, but its own repo
  3. Docs can be updated as part of the same PR that updates the respective code, and arbitrary checkouts of the repo will always contain the docs valid for that version

On a personal note, I much prefer editing text in an editor I'm comfortable with, and Browsers don't fall into that category.

I'm curious to hear your thoughts on this proposed change.

@har7an har7an requested a review from a team as a code owner February 6, 2024 08:08
Copy link
Member

@Emilgardis Emilgardis left a comment

Choose a reason for hiding this comment

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

Awesome job! Some nits but looks great, thank you :)

This makes me also want to migrate to a mdbook, which we probably should've done from the start...

I'm wondering now though, we should probably edit the wiki with this

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
docs/custom_images.md Outdated Show resolved Hide resolved
docs/config_file.md Show resolved Hide resolved
docs/config_file.md Outdated Show resolved Hide resolved
README.md Show resolved Hide resolved
docs/environment_variables.md Outdated Show resolved Hide resolved
@Emilgardis Emilgardis added the no changelog A valid PR without changelog (no-changelog) label Feb 6, 2024
@har7an har7an force-pushed the docs/restructure-docs-folder branch from 716f129 to fc4debb Compare February 13, 2024 05:38
@har7an
Copy link
Contributor Author

har7an commented Feb 13, 2024

This makes me also want to migrate to a mdbook, which we probably should've done from the start...

Weeeell, I can work out a proposal if you like. In case I do step up to the task: Should we do that in a separate PR?

I'm wondering now though, we should probably edit the wiki with this

Is it really worth the effort in the long run? Especially when moving towards mdbook for documentation, wouldn't it be easier for everyone if there were only one source of documentation for the entire project?

from the Wiki that details how to get started with using `cross`.
and remove the respective section from the README instead.
into two separate files in the `docs/` dir and enrich it with
information taken from the Wiki.
with additional information from the wiki. Split up documentation in a
new file in the `docs/` directory and add references to other files
where appropriate.
@har7an har7an force-pushed the docs/restructure-docs-folder branch from fc4debb to f37f180 Compare February 13, 2024 05:54
@har7an
Copy link
Contributor Author

har7an commented Feb 13, 2024

In e636ee5 I've made the cross_toml module in the Rust sources public to eliminate some warnings during building the docs. Now the cross_toml module shows up in the crate documentation, pulling in the content of docs/config_file.md as its' module docs.

While I think it's nice to have that part of the docs mirrored there, unfortunately all the "external" links to other MD docs are broken there. How should we proceed? Imo we have several options:

  1. Keep the current state with the broken links
  2. Revert the changes, make cross_toml a private module again and remove the #![doc] directive completely
  3. Use something like rustdoc_assets to copy the docs/ folder as static assets into the generated docs, making the links in (1) work

Approach 3 seems to rely on build files, tho. I see you're already using xtask, but I have no idea to what extent we can influence how the docs on docs.rs are built, i.e. whether it always runs the same static command, or whether we can tell it to use xtask. Since afaik the docs are built without network access, though, my assumption is that using xtask (which has to compile itself) is out of scope... :(

@Emilgardis
Copy link
Member

Emilgardis commented Feb 13, 2024

This makes me also want to migrate to a mdbook, which we probably should've done from the start...

Weeeell, I can work out a proposal if you like. In case I do step up to the task: Should we do that in a separate PR?

That can be done separately. I opened #1446 to track it

I'm wondering now though, we should probably edit the wiki with this

Is it really worth the effort in the long run? Especially when moving towards mdbook for documentation, wouldn't it be easier for everyone if there were only one source of documentation for the entire project?

What I meant with editing the wiki is simply just to make it say on each page "see docs/<page> for documentation regarding ..."

While I think it's nice to have that part of the docs mirrored there, unfortunately all the "external" links to other MD docs are broken there. How should we proceed? Imo we have several options:

I think we can just remove the doc from the rustdoc, it's not needed and we can instead just point to the files in the crate package. The only reason its done was because the cross_toml.md file used to be the module documentation for that file bc54c2a#diff-a93d7b71ff09816ff274b195eaafc678ec01ab637abfe3ce49c0a14dfec32913

@har7an
Copy link
Contributor Author

har7an commented Feb 20, 2024

Sorry for the slow responses!

What I meant with editing the wiki is simply just to make it say on each page "see docs/<page> for documentation regarding ..."

I see. Will you do that once this is merged, or should I?

I think we can just remove the doc from the rustdoc, it's not needed and we can instead just point to the files in the crate package.

Alright, got it. Now I have:

  1. Made cross_toml a private module again and removed the docs directive from the top, so it has no module docs anymore.
  2. Added a reference to the docs/ directory in the repo to the "root" crate docs (next to README and wiki).
  3. Fixed a remaining link pointing to Cross.toml, which now points to the config_file.md file instead.

What do you think?

Copy link
Member

@Emilgardis Emilgardis left a comment

Choose a reason for hiding this comment

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

Changes look great except the "broken" link being fixed.

Aside from that, I think this is ready for being merged, do you want to squash/fixup the commits you've marked as squash! or fixup!?

src/lib.rs Outdated
Comment on lines 902 to 905
/// Obtains the [`CrossToml`][1] from one of the possible locations
///
/// These locations are checked in the following order:
/// 1. If the `CROSS_CONFIG` variable is set, it tries to read the config from its value
Copy link
Member

Choose a reason for hiding this comment

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

Hmm, this shouldn't be a broken link, it's a intra-doc link, please revert this

@har7an har7an force-pushed the docs/restructure-docs-folder branch from b49c238 to f802493 Compare February 20, 2024 07:57
to improve readability.
taken from the wiki with some added information.
which is now superseeded by the new arrangement of files in the `docs/`
directory.
and explicitly point towards environment-variable based configuration
which was previously missing. This should make that part of the
documentation easier to find.
and limit text to 80 columns. Move a few hyperlinks to the bottom of the
file rather than spraying them in between text passages.
fix a renamed TOML table key and update suboptimal linebreaks.
to make sure it's more visible and deduplicate installation examples
with the `CROSS_DEB_ARCH` variable.
instead of the `cross_toml` file that has been replaced as part of this
PR.
and make sure that `cross_toml` shows up as part of the official
documentation along the way.
@har7an har7an force-pushed the docs/restructure-docs-folder branch from f802493 to 936804e Compare February 20, 2024 08:05
@har7an
Copy link
Contributor Author

har7an commented Feb 20, 2024

@Emilgardis That first rebase was a little too eager, sorry... Changes are in now!

src/cross_toml.rs Outdated Show resolved Hide resolved
to point to the original repo instead of the fork for this PR.
Copy link
Member

@Emilgardis Emilgardis left a comment

Choose a reason for hiding this comment

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

Thank you for this! I'll edit the wiki to point to these documents

@Emilgardis Emilgardis added this pull request to the merge queue Feb 20, 2024
Merged via the queue into cross-rs:main with commit c87a52a Feb 20, 2024
22 checks passed
@har7an har7an deleted the docs/restructure-docs-folder branch February 20, 2024 10:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
no changelog A valid PR without changelog (no-changelog) no-ci-tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants