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 typedoc, test doc generation #588

Merged
merged 35 commits into from
May 21, 2024
Merged

add typedoc, test doc generation #588

merged 35 commits into from
May 21, 2024

Conversation

tomasciccola
Copy link
Contributor

@tomasciccola tomasciccola commented Apr 25, 2024

This should close #586

In order for this documentation to be useful, it would be nice to know expectations for this (what things do we think are necessary to make this useful?).
For now I:

  • set up a plugin to render it as markdown
  • commit doc files so they can be viewed directly here

@EvanHahn
Copy link
Contributor

What if we could build these to digidem.github.io/mapeo-core-next using CI, rather than committing them to the doc/ folder?

@tomasciccola
Copy link
Contributor Author

What if we could build these to digidem.github.io/mapeo-core-next using CI, rather than committing them to the doc/ folder?

Yeah, that could be an option. I really don't have a strong opinion on this.
Do you see a disadvantage on committing docs to the repo?
Having a dedicated page for typedoc kinda makes sense. But I also like having documentation in markdown directly on the repo.
A middle ground could be:

  • commit docs on the repo as markdown
  • have that same documentation published as gh-pages (this may make sense to set up using CI, since we need to generate docs as html, not md)

keen on other's opinion @achou11 @gmaclennan

@achou11
Copy link
Member

achou11 commented May 2, 2024

I was writing a decently long response to answer the questions here but we just discussed this over a call, so will document what was agreed instead:

  • Stick to committing the generated docs to the repo and deploying the directory using repo-specific github pages feature. we want to avoid touching the org-specific github pages because there's a preexisting site there (repo).
  • Adjust the configuration for the generator so that it only generates documentation for exported APIs. Having all of the internal stuff is not as useful in the short-term for us
  • Figure out whether we want docs to be updated per PR/commit or if there's some kind of automation we use to update the docs after PRs are merged in. Would be nice to set up a workflow that can provide a "preview" url (similar to Netlify, Vercel, etc) for a PR.

@tomasciccola
Copy link
Contributor Author

Following andrews proposal I've basically set up documentation so that only public things gets docgenerated and commited to the repo. On the other hand I've set up a workflow so that we can publish an html version of documentation to ghpages.
This is now deployed here. This is a custom domain set up for the repo. I think the idea is not to hosted there, but on the default url (so, smth like mapeo-core-next.githab.pages.io ? ).
But I can't seem to find a way to ignore a custom domain for a specific page.
I'm wondering if we can delete the custom domain setup for this repo (I think so since I don't think is being use for anything else in this repo) so it can be deployed to the default url.
Regarding WHEN to generate the docs. For now is set up so that only updates on the main branch deploy the docs (which I think it makes sense?). After merging this PR I need to remove a protection rule so that this branch can also deploy the page.
The thing that I may need some help setting up is the preview of the page. What I'm thinking is that this preview should be accessible on a PR that is going to be merged to main. For that I may need to generate a token (gonna look at this a little bit more but that seems to be the case...)

@tomasciccola
Copy link
Contributor Author

Also, along side MapeoManager (which is kinda the the thing we want to generate docs for) there are some other stuff exported mostly related to fastify. We can explicitly ignore those classes (ussing a @hidden tag in the jsdoc) or just leave them there...

@EvanHahn
Copy link
Contributor

EvanHahn commented May 8, 2024

This is looking good!

I'm wondering if we can delete the custom domain setup for this repo (I think so since I don't think is being use for anything else in this repo) so it can be deployed to the default url.

Let me know if you want me to do this.

Regarding WHEN to generate the docs. For now is set up so that only updates on the main branch deploy the docs (which I think it makes sense?). After merging this PR I need to remove a protection rule so that this branch can also deploy the page.

I think it makes sense to generate every time main changes.

After merging this PR I need to remove a protection rule so that this branch can also deploy the page.

I wonder if we can disable the branch protection only for the GitHub Actions bot. I see this in the settings:

Screenshot

The thing that I may need some help setting up is the preview of the page.

I think this can be done in a followup PR.

Also, along side MapeoManager (which is kinda the the thing we want to generate docs for) there are some other stuff exported mostly related to fastify. We can explicitly ignore those classes (ussing a @hidden tag in the jsdoc) or just leave them there...

I also think we should do this in a followup.

@achou11
Copy link
Member

achou11 commented May 8, 2024

not sure if disabling the custom domain for this repo will be trivial/possible based on what's described here:

https://docs.github.com/en/pages/configuring-a-custom-domain-for-your-github-pages-site/about-custom-domains-and-github-pages#using-a-custom-domain-across-multiple-repositories

we have the custom domain set up on our org , which means that all of the projects are subject to it. in order to override it, we need to point to a new custom domain, but I don't think we can "revert" it to the project-specific default of digidem.github.io/mapeo-core-next

@achou11
Copy link
Member

achou11 commented May 8, 2024

Gregor mentioned that he's set up https://core.comapeo.app/ to point to GH pages so can use that for this project

@tomasciccola
Copy link
Contributor Author

tomasciccola commented May 14, 2024

I think it'd be cool to do this when we publish a new version, probably as part of our existing release script (see .github/workflows/release.yml).

I've added a flow to release.yml to basically do this by using git and commiting the files. I've also added it to the docs.yml flow so that we can test without needing to do a release (will probably remove that step before merging though...)

Currently I'm using a plugin typedoc-plugin-missing-exports that basically includes files that are referenced by a public file. This makes documentation more useful (i.e., you can see MapeoProject documentation now). The plugin doesn't have any options the allow smth like defining how deep in the dep chain it goes, so it kinda includes everything that is referenced...

👍 Would be cool to fix this somehow in the future, not sure how.

Yeah, me neither, same with all the internal files that are named with an absolute path...

@tomasciccola tomasciccola requested a review from EvanHahn May 14, 2024 17:29
@tomasciccola
Copy link
Contributor Author

tomasciccola commented May 14, 2024

Things to pay attention to:

  • Security implications of giving gh-action permission to write the repo (can this be avoided by setting a different git user - like one of us - in the action itself?)
  • improving the flow by using additional parameters (see git-auto-commit-action options)

@EvanHahn
Copy link
Contributor

EvanHahn commented May 14, 2024 via email

npm run doc
git status
git add docs/api/md
git commit -am "updated api docs"
Copy link
Contributor

Choose a reason for hiding this comment

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

Two nitpicks:

  1. Can we move this to the conventional commits style?
  2. -a adds all files, which I think we'd rather not do.
Suggested change
git commit -am "updated api docs"
git commit -m "docs: update API docs"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, this became out of sync with the actual flow that uses an external action that actually commits the file (see the docs.yml flow). I'll set the message so that it uses conventional commits

Comment on lines 30 to 34
- name: Push updated markdown docs
uses: stefanzweifel/git-auto-commit-action@v5
with:
commit_message: 'Update api markdown docs'
file_pattern: 'docs/api/md/*'
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we want to update the Markdown docs after every commit to main? I feel like...maybe not? I like that you've added it as part of the release task.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

nnna, this was just to test doc generation during dev

Copy link
Contributor

Choose a reason for hiding this comment

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

Would you mind removing this, then?

Copy link
Contributor

@EvanHahn EvanHahn left a comment

Choose a reason for hiding this comment

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

LGTM other than my two small nitpicks.

.github/workflows/release.yml Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
@tomasciccola tomasciccola merged commit c8dbdc3 into main May 21, 2024
8 checks passed
@tomasciccola tomasciccola deleted the feat/typedoc branch May 21, 2024 18:50
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.

Set typedoc to generate documentation from types
3 participants