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/copy edit 1 #315

Merged
merged 4 commits into from
Sep 12, 2024
Merged

Docs/copy edit 1 #315

merged 4 commits into from
Sep 12, 2024

Conversation

kmurphypolygon
Copy link
Contributor

  • This PR does a quick scan and update for style guide.
  • Please note only one # (H1) per page.
  • Also added back the requirements.txt and run script for easy spin up of docs.
  • Fixed a link.

requirements.txt Outdated
Copy link
Contributor

@bobbinth bobbinth Sep 11, 2024

Choose a reason for hiding this comment

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

Quick comment: we usually put this file as well as run.sh into scripts directory and name them serve-doc-site.sh and docs_requirements.txt (e.g., see here).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi there, I noticed you didn't have them in this repo, so I just dropped in what we normally use in the KL repo and others without rewriting anything. Ideally, we should have a common process across all doc sites so as not to confuse tech writers but feel free to customize this as you wish and if you do customize please add some instructions on how you prefer to spin up the site :)

Copy link
Contributor

@bitwalker bitwalker Sep 12, 2024

Choose a reason for hiding this comment

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

FYI: These files are already present in the docs directory, and the docs can built/run using cargo make book or cargo make serve-book depending on whether you want to run it live or not.

EDIT: You can also use docs/mkdocs build or docs/mkdocs serve directly, cargo make just wraps that script in our build system

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok, cool ... that's great, please go ahead and remove the files I dropped in, no problem

Copy link
Contributor

@bitwalker bitwalker left a comment

Choose a reason for hiding this comment

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

LGTM! However, I'd like to remove the run.sh and requirements.txt in favor of the ones that already exist in the repo under the docs directory before merging. I can make that change, as well as make sure our docs/README.txt explains how to build and run the documentation using either of the two methods I mentioned in my other comment - unless you'd prefer to do so, just let me know!

@@ -124,7 +123,7 @@ will be expanding the SDK rapidly over the next few weeks and months, but for th
you encounter a missing API that you need, let us know, so we can ensure it is prioritized above
APIs which are lesser used.

### Rust/Miden FFI (Foreign Function Interface) and Interop
### Rust/Miden FFI (foreign function interface) and interop
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't "foreign function interface" remain capitalized here, to emphasize the correlation to the initials of the "FFI" acronym?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@@ -3,3 +3,15 @@ bin/midenc
bin/litcheck
.crates.toml
.crates2.json

# Docs platform ignores
Copy link
Contributor

Choose a reason for hiding this comment

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

Note: Our docs build in the target directory, which is already ignored, and our docs directory has a dedicated .gitignore for things within that directory, so most of these directives will never be matched.

That said, I'm fine with adding these here anyway (mostly the IDE-specific directives, since we should have those anyway), just in case someone accidentally builds the docs in the project root locally - but just wanted to clarify where you can expect these things to actually be output in this repo.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, whenever I spin it up locally and make changes, I'm prompted to push a whole load of unnecessary stuff ... so that's why I added these like we have in the Miden base repo.

@kmurphypolygon
Copy link
Contributor Author

kmurphypolygon commented Sep 12, 2024

Just a quick note please @bitwalker .. we have an error on main docs

The following pages exist in the docs directory, but are not included in the "nav" configuration:
             - miden/miden-compiler/appendix/known-limitations.md

This file will remain live and searchable even if there is no menu link to it ... so I'm not sure it that's what you need? Or perhaps you need to add a menu link?

@bitwalker
Copy link
Contributor

@kmurphypolygon I've added the missing nav item for that page, build instructions for the docs, and the tweaks I suggested, in the latest commit. I'm going to go ahead and merge this, but if you spot any other issues or have other suggested changes, let me know!

@bitwalker bitwalker merged commit 83e234b into next Sep 12, 2024
4 checks passed
@bitwalker bitwalker deleted the docs/copy-edit-1 branch September 12, 2024 14:56
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.

3 participants