Skip to content

Conversation

@daj-code
Copy link
Contributor

@daj-code daj-code commented Dec 22, 2020

Why this change?

Navigating the floki documentation is currently non-trivial, as the format of the documentation is a "blog" and therefore has no clear structure or navigation. Instead content is ordered by date created/modified (as set in the page metadata), and the only form of navigation is from the README.md for the project. Also, to test the documentation after editing it, or to test building it after making changes, is not straightforward and requires you to install the tool hugo.

This PR aims to add three key improvements to the floki documentation:

  1. Make it quick and easy to get an environment for building and testing the floki documentation. This seems like a clear example use case for floki! Therefore, this is done by adding a floki-hugo.yaml config file to the repository to quickly launch a lightweight container where this building and testing of documentation is possible. In particular, this also allows you to quickly launch a hugo server to serve the pages locally, which are then accessible at http://localhost:1313/floki/. The only caveat with this is that it currently uses a much more recent version of hugo than is currently used in Travis CI to build the published documentation.
  2. Add a navigation section to the documentation that is weighted to always appear at the top of the documentation homepage. This then links to the relevant sections of the documentation. As discussed in WIP: Generating a configuration reference #105, it seems that a larger overhaul of the documentation may want to happen at some point, but this provides an improvement to documentation navigability (even if that is a temporary one).
  3. Add code language to code blocks in README.md and documentation. This allows syntax highlighting when available, which can improve readability of the code blocks. I also added some sections from README.md to the actual documentation (as they were missing) such as installation via cargo being possible.

I have also added a couple of checks to the PR template to help ensure that documentation is remembered to be kept up to date in future. In addition, I have fixed a few typos/spelling errors I noticed in passing.

Relevant testing

I have tested the documentation by using the new floki-hugo.yaml config to run floki and then inside that container I ran hugo server -D to serve the edited documentation at http://localhost:1313/floki/, where I was able to verify the changes that had been made looked as I expected them to. In particular, I verified that the new navigation section always appeared at the top of the documentation homepage, even when the date set in the page metadata was before other pages (and therefore would have appeared lower down the documentation). I also tested that the links in the new navigation page function correctly.

Contributor notes

I have added the .vscode folder to .gitignore, as I use VSCode for editing and having to consistently remember to not include this in commits was cumbersome. If you'd rather keep the .gitignore file clean of editor-specific artifacts, I am happy to remove this change from the PR. (REMOVED)

Checks

  • New/modified Rust code formatted with cargo fmt
  • Documentation and README.md updated for this change
  • CHANGELOG.md updated for this change

…ilding and testing documentation.

Signed-off-by: David Jackson <[email protected]>
… off relevant sections.

Because of the theme used (to look like a blog post) the documentation has no clear navigation. This new document helps rectify that without changing the whole theme.

Signed-off-by: David Jackson <[email protected]>
- Fix two typos in code

- Add syntax to code blocks in documentation and other markdown documents

- Add vscode folder to git ignored folders

Signed-off-by: David Jackson <[email protected]>
These should help catch common things missed in PRs that mean that e.g. documentation gets out of sync with reality.

Signed-off-by: David Jackson <[email protected]>
@rlupton20
Copy link
Collaborator

Hi David,

thanks for doing this.

One quick nit (I've not looked in detail), vscode related gitignores probably belong in your global gitignore (I do the same for emacs related guff). Editor specifics probably belong out of project codebases, and in your personal configuration instead.

Otherwise looks good! Feel free to assign me to do a proper review when you are ready.

…long in the user's global .gitignore file.

Signed-off-by: David Jackson <[email protected]>
@daj-code
Copy link
Contributor Author

Thanks for the feedback @rlupton20 - I wasn't previously aware of the global .gitignore file, so that definitely makes more sense for these editor specifics, thanks! I've removed the change from the PR in the most recent commit.

I don't seem to be able to assign the PR to you, but it's ready for review whenever you've got chance to look at it.

@rlupton20 rlupton20 self-requested a review December 23, 2020 22:25
Copy link
Collaborator

@rlupton20 rlupton20 left a comment

Choose a reason for hiding this comment

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

Looks good - thanks for doing this!

@rlupton20
Copy link
Collaborator

Thanks for the feedback @rlupton20 - I wasn't previously aware of the global .gitignore file, so that definitely makes more sense for these editor specifics, thanks! I've removed the change from the PR in the most recent commit.

No worries, I didn't know about it for quite a while :-).

@daj-code
Copy link
Contributor Author

daj-code commented Jan 3, 2021

Thanks for reviewing @rlupton20, I've applied the markup you suggested. I see you've approved the PR, but would you like to re-review?

@rlupton20
Copy link
Collaborator

@daj-code sorry I missed your message - it must have gotten lost in the noise. I already took a look over your new changes - go ahead and merge if you are ready :-).

Thanks again for doing this work, and sorry about the long delay in response!

@daj-code
Copy link
Contributor Author

daj-code commented Feb 3, 2021

No worries @rlupton20. I don't think I have permission to merge, so would you be able to do so? Thanks!

@rlupton20 rlupton20 merged commit c59a0d8 into Metaswitch:master Feb 3, 2021
@rlupton20
Copy link
Collaborator

Yeah, sorry, bloody permissions...

@daj-code daj-code deleted the daj/better-docs branch February 3, 2021 22:38
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.

2 participants