-
Notifications
You must be signed in to change notification settings - Fork 10
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
base: main
Are you sure you want to change the base?
Conversation
…ng to Github Pages,
…e style and grammar
…d missing packages for SVG export
- and minor edit to README clarity
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.
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!).
Co-authored-by: glopesdev <[email protected]>
Sound good, thanks for reviewing it! Accepted almost all the edits, and left comments on a few of the issue to be discussed. |
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.
@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.
@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. |
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 |
@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 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. |
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.
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). |
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.
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.
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.
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?
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.
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 |
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.
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).
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.
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?
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.
Hmm I am not sure how to answer this, but I think there are two separate issues to consider here:
- 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?
- 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. |
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.
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."
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.
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. |
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.
interoperate deeply sounds a bit funny to me, maybe "when packages require extensive integration with other packages?"
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.
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. |
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.
"possible" is doubled twice here
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."
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."
"We might want to consider marking the Tutorials section as optional for other package documentation, to make things simpler as a common reference."
"Review format of relative / local URLs to make sure we have a solution that works for both local and remote builds."
../
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."
Other things that I have changed or moved around since the last time:
Online preview of this PR and PRs #97 and #94 : https://banchan86.github.io/bonsai_docs_test/articles/documentation-docfx.html