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

Add docfx and documentation style guide #103

Open
wants to merge 31 commits into
base: main
Choose a base branch
from

Conversation

banchan86
Copy link
Contributor

@banchan86 banchan86 commented Sep 19, 2024

Followup to #82, resolves #102.
Accidentally closed the original PR as I was shifting things around but have reproduced the comments below.
This PR introduces two new articles a "Documentation with docfx" and "Documentation style guide"

Review breakdown:

  • "We might want to consider mentioning here how to build the solution in order to export the example workflows, and in that case also consider how to automate the msbuild part into the powershell module containing the export image functions."

    • I have included sections on creating a local environment, setting up build.ps1 in the documentation docfx article, and added more detail to the Bonsai workflow section of the documentation style guide. Its a little over the place, and I think I placed things where they would logically belong, but happy to revise. Im not sure I will be able to carry out 2nd part of that comment.
  • "We should maybe include these as part of docfx-tools or some other template repo."

    • I initially raised a PR to include them as part of the docfx-tools submodule but i realized that maybe wasn't the best solution as users would still need to modify and move them. I am more in favor of hosting them on the docfx-assets repository for users to download but not import as a submodule and have raised a PR to add them. I have also updated the documentation articles with that strategy in mind.
  • "We might want to consider marking the Tutorials section as optional for other package documentation, to make things simpler as a common reference."

    • I have marked this more explicitly as being optional in the top section of the documentation style guide
  • "Review format of relative / local URLs to make sure we have a solution that works for both local and remote builds."

    • So I went to research this and from what I can tell its safer to use ../ rather than ~/ and given that our scripts/code use ../ for navigation, I thought it would be best to standardize on using ./ and ../. I have tested this on docfx articles and it works and I have changed instances of ~/ as well as backward slashes in the documentation articles to reduce ambiguity.
  • "It might be worth mentioning as a note the importance of matching the publishing URL with the PackageProjectUrl attribute specified in the .csproj files. This is where the Bonsai editor will look for the xrefmap.yml file to retrieve URLs for operators."

    • Have added a note to the publishing to github pages section.

Other things that I have changed or moved around since the last time:

  • Decided to use directory trees to better represent file locations and simplify the text in those sections
  • Updated the section that deals with the local bonsai environment given that I now have a better understanding of how that works.
  • Restructured the content in a way I think makes more sense (that lead to the creation of the repository organization, doc organization and version control cleanup sections)
  • Expanded the text so that it reads less like a README and more like an article with proper explaination for why we do certain things, but I may have taken it too far and made it too wordy :)

Online preview of this PR and PRs #97 and #94 : https://banchan86.github.io/bonsai_docs_test/articles/documentation-docfx.html

- and minor edit to README clarity
@banchan86 banchan86 changed the title Newdoc guide Add documentation with docfx and documentation style guide Sep 19, 2024
@banchan86 banchan86 changed the title Add documentation with docfx and documentation style guide Add docfx and documentation style guide Sep 19, 2024
@glopesdev glopesdev self-requested a review November 8, 2024 16:24
@glopesdev glopesdev added the documentation Improvements or additions to documentation label Nov 8, 2024
Copy link
Member

@glopesdev glopesdev left a comment

Choose a reason for hiding this comment

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

Thanks for putting this together @banchan86, it's a really great start. I did a full first pass over the two guides, this is definitely something we want to have merged soon.

The only thing I am still struggling with is how to move with general infrastructure decisions such as docfx-assets, the centralization or decentralization of package documentation, package documentation index and discoverability, etc.

I will try to make some definite progress on this so we can potentially review this soon, but in the meantime I'm leaving some general feedback that can be worked on independently.

If we can't resolve some of these fundamental questions soon we may want to merge this anyway and iterate later, since this is clearly much better than what we have now (which is either nothing or obsolete content!).

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
README.md Outdated Show resolved Hide resolved
articles/documentation-style-guide.md Outdated Show resolved Hide resolved
articles/documentation-style-guide.md Outdated Show resolved Hide resolved
articles/documentation-style-guide.md Outdated Show resolved Hide resolved
articles/documentation-style-guide.md Outdated Show resolved Hide resolved
articles/documentation-style-guide.md Outdated Show resolved Hide resolved
@banchan86
Copy link
Contributor Author

banchan86 commented Nov 9, 2024

Thanks for putting this together @banchan86, it's a really great start. I did a full first pass over the two guides, this is definitely something we want to have merged soon.

The only thing I am still struggling with is how to move with general infrastructure decisions such as docfx-assets, the centralization or decentralization of package documentation, package documentation index and discoverability, etc.

I will try to make some definite progress on this so we can potentially review this soon, but in the meantime I'm leaving some general feedback that can be worked on independently.

If we can't resolve some of these fundamental questions soon we may want to merge this anyway and iterate later, since this is clearly much better than what we have now (which is either nothing or obsolete content!).

Sound good, thanks for reviewing it! Accepted almost all the edits, and left comments on a few of the issue to be discussed.

Copy link
Member

@glopesdev glopesdev left a comment

Choose a reason for hiding this comment

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

@banchan86 thanks, all looks good to me, I just noticed a few opportunities to use relative instead of absolute links and after that it should be good to go.

Other than the files in this PR, the only other absolute link I noticed in a cursory look through the entire docs is Line 181 of closed-loop.md where [property mapping operators](http://bonsai-rx.org/docs/property-mapping){:target="\_blank"} could be replaced with [property mapping operators](../articles/property-mapping.md)..

We can also do that one on a separate PR but since it is just the one off maybe there is no harm in adding it here.

articles/documentation-docfx.md Outdated Show resolved Hide resolved
articles/documentation-docfx.md Outdated Show resolved Hide resolved
articles/documentation-docfx.md Outdated Show resolved Hide resolved
articles/create-package.md Show resolved Hide resolved
articles/documentation-style-guide.md Outdated Show resolved Hide resolved
@glopesdev
Copy link
Member

glopesdev commented Nov 15, 2024

@banchan86 Thanks for the edits, almost there, I have added a final round of suggestions throughout in a final commit directly to the PR branch (4457090), let me know if these look good to you. There are a few things which might still change in the docs short-term but I think I'm happy to push this version and then iterate as we improve.

@banchan86
Copy link
Contributor Author

Looks good to me! Before pushing it though, have we decided what to do with the documentation assets? As the articles point to some files that don't currently exist yet in the docfx-assets repo unless bonsai-rx/docfx-assets#2 is merged in as well.

@glopesdev
Copy link
Member

glopesdev commented Nov 27, 2024

@banchan86 good point about the assets. What I decided to do is actually embed them directly in the guide as code sections, similar to the bits about configuring docfx.json. Turns out most of them are really small and easy to understand, except for the GitHub Actions workflow.

For that one it actually gets even more complicated since we don't have yet a solid standard of how automation should be made, so I deferred with a link to an example workflow for now. We can review later when we have something more refined and stable.

If this sounds good I would be happy merging this.

Copy link
Contributor Author

@banchan86 banchan86 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 to me overall, and I agree with the approach of just embedding the code snippets for most of the files, but not sure about pointing to the main docs GitHub actions recipe.

@@ -28,24 +28,24 @@ dotnet new install Bonsai.Templates
dotnet new bonsaienv
```

- `.github/workflows/` - The `docfx` website is published to [GitHub Pages](https://pages.github.com/) using a [GitHub Actions](https://docs.github.com/en/actions/writing-workflows/workflow-syntax-for-github-actions) workflow called `docs.yml`. Download this folder from the [docfx-assets](https://github.com/bonsai-rx/docfx-assets) repository and amend `Bonsai.PackageName` to point to your package source code in `docs.yml`.
- `.github/workflows/` - The `docfx` website is often automatically published to [GitHub Pages](https://pages.github.com/) using a [GitHub Actions](https://docs.github.com/en/actions/writing-workflows/workflow-syntax-for-github-actions) workflow specified in the `docs.yml` file. Unfortunately these workflows tend to be very project specific and we do not yet have a standardized pipeline. For a minimal example and inspiration you can check [the workflow for this repository](https://github.com/bonsai-rx/docs/blob/main/.github/workflows/build.yml).
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am ok with pointing to a sample repository for the GitHub Actions recipe, my main concern with pointing to the bonsai docs repo 'build.yml' is that compared to most of the other packages, it's missing the parts of the pipeline for building and executing the docfx-tools pipeline for SVG export on GitHub Actions.

Copy link
Member

Choose a reason for hiding this comment

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

That is a good point, although unfortunately most of the other repos are also using deprecated actions which I would prefer not to use as a reference going forward. I was wondering maybe the best middle ground would be to upgrade one of the old repos to try out the upgraded pipeline.

I am reviewing bonsai-rx/zeromq#22 and will try to upgrade everything to fit correctly. Once we are done we can point to that, does that sound good?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sounds good to me!

@@ -250,32 +283,32 @@ To keep your online GitHub repository clean, you can use `.gitignore` files to i
...
```

## Testing unpublished packages
## Creating example workflows
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmm I am not sure about the title change here, as it changes the emphasis and function of this section. For contributing to documentation for published packages there is no need to go through the visual studio pipeline. and a separate section on saving Bonsai workflows in the documentation style guide (./documentation-style-guide.md#bonsai-workflows).

Copy link
Member

Choose a reason for hiding this comment

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

Good point about the duplicate section. I think I would rather move everything to one of the sections then. Perhaps this one, since including example workflows feels really part of the core documentation infrastructure and not simply a style recommendation. Does that sound good?

Maybe I am confused about the distinction between "published" and "unpublished" packages. I was imagining that documentation would always be done targeting the source code of an existing package, and so in a way you always treat it as "unpublished" since you might be documenting a new release.

Although perhaps you were thinking of cookbook style or recipe examples and tutorials which might live outside of any package and where you just describe in detail how to use existing packages to achieve a specific goal? In that case I can see the difference, but then the entire way of setting up docfx might be different anyway and potentially deserve a separate guide? For now maybe we just explain how to document custom packages?

Copy link
Contributor Author

@banchan86 banchan86 Nov 28, 2024

Choose a reason for hiding this comment

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

Hmm I am not sure how to answer this, but I think there are two separate issues to consider here:

  1. Unpublished vs published packages

I guess the distinction for me was writing documentation for new releases like Bonsai.PulsePal vs existing releases like BonVision.

It was a pain point for me when i started working on the Bonsai.PulsePal documentation because I initially did not know how to test the package out (which at that time was not released yet). I took what I learnt from the 'Creating a Package' guide and decided to write up this short section in case it helped other documentation contributors who were not package developers. However I do think that maybe it is a little out of place here and package developers writing documentation for their own packages would already have this knowledge. In that case maybe this section is unnecessary?

  1. Bonsai workflow examples

I think consolidating the instructions for how to generate Bonsai example workflows might be a good idea as I do think its part of the core documentation infrastruture. I guess I was just struggling with how to organise this content and chose to document the import of the docfx-tools submodule as part of the docfx infrastructure and the saving of the Bonsai examples workflows as part of the media/content recommendations.

In that case, I would probably make a new separate section in this article and title it "Bonsai workflow support" and pull in the content about the importing of the docfx tools submodule, where to save the Bonsai workflows, and how to use the 'build.ps1' script. That leaves the markdown instructions for how to include them in articles. It might still be a good idea to leave in the style guide, along with a link to this "Bonsai workflow support" section to ensure that people set up the infrastructure correctly. If thats ok with you, I could do a rewrite in a new commit (unless you want to organise it yourself differently?)

2. Install Bonsai VS Extensions. Assuming Bonsai is already installed, from the Windows Start Menu, search for the "Install Bonsai VS Extensions" shortcut and run it.
3. In Visual Studio, open `src/PackageName.sln` in the repository.
4. Press F5 to open the Bonsai editor with the new package added.
5. From here, you can make Bonsai workflows and save them to the `workflows` folder as examples.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Might be good to throw a link to the bonsai workflow container section and also change the text a little bit to reinforce the function of this section. Suggested change - "From here, you can test the package and save Bonsai workflows as examples."

Copy link
Member

Choose a reason for hiding this comment

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

Good point, I will rephrase.

* Tutorials - hosts examples or tutorials for various applications. This section is optional, but valuable for more complicated applications or packages which require operators from other packages for their execution.
* `Manual` - articles explaining the core concepts of the package and functions of its various operators.
* `Reference` - technical documentation for each operator, generated automatically by `docfx` from XML comments in source code or supplemented with individual operator articles.
* `Tutorials` - examples or tutorials about using the package. This section is optional, but valuable for more complex applications or when packages interoperate deeply with other packages.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

interoperate deeply sounds a bit funny to me, maybe "when packages require extensive integration with other packages?"

Copy link
Member

Choose a reason for hiding this comment

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

Thanks, I will rephrase.


### Figures

> [!NOTE]
> Avoid images/screenshots when possible as they do not display well across light/dark mode and do not scale well across different display sizes and resolutions. See the following sections for alternative ways of creating different content.
> Avoid images/screenshots as much as possible possible as they often do not display well across light/dark mode and do not scale well across different display sizes and resolutions. See the following sections for alternative ways of creating different content.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

"possible" is doubled twice here

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Create articles on docfx and documentation style guide
2 participants