-
Notifications
You must be signed in to change notification settings - Fork 68
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
API docs #1697
API docs #1697
Conversation
@@ -2,7 +2,9 @@ | |||
* Copyright 2021 TypeFox GmbH | |||
* This program and the accompanying materials are made available under the | |||
* terms of the MIT License, which is available in the project root. | |||
******************************************************************************/ |
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.
It seems that TypeDoc was not able to parse this comment ending properly, so I reduced it to the normal */
. If this is a big deal, I can create an issue and wait until it gets resolved.
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.
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.
Yeah both sound fine to me, interesting that typedoc couldn't pick up the longer closing block comment.
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.
Yup, perhaps I should report it as a bug
Looks great! |
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.
Nice! I just have a few questions in here, but overall it's looking really good (and the preview by extension as well).
In particular I wanted to say thanks for cleaning up some of the old doc comments, especially with respect to params that are no longer present.
.github/workflows/publish.yml
Outdated
|
||
jobs: | ||
publish: | ||
name: Langium Publish | ||
runs-on: ubuntu-latest | ||
timeout-minutes: 20 | ||
if: ${{ github.event_name == 'release' }} |
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 don't know if we want to constrain the publish job like this. We didn't need it before, and I think we may want to run this under different circumstances than this constraint would allow.
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.
Because we didn't have any other trigger before, but now I added the workflow_dispatch
trigger to allow updating the docs manually even before a release. Now that I think about it, I'm not sure if this would actually ever be used
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, yeah I'm still not sure about this one. @msujew thoughts here?
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 think it makes more sense to have a separate workflow_dispatch
job for the API docs publishing - right now, the release publishing process is broken due to expired credentials.
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.
By "separate job", do you mean an entirely separate .yml file or just the additional trigger here?
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.
A separate yml file :)
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.
done
@@ -2,7 +2,9 @@ | |||
* Copyright 2021 TypeFox GmbH | |||
* This program and the accompanying materials are made available under the | |||
* terms of the MIT License, which is available in the project root. | |||
******************************************************************************/ |
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.
Yeah both sound fine to me, interesting that typedoc couldn't pick up the longer closing block comment.
My pleasure, and that's another benefit of using TypeDoc, it emits warnings whenever there is a discrepancy between the docs and the implementation. |
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.
nice this looks great. Thanks a ton @aabounegm ! I have no problems on my end with bringing this in now.
The other question is when we should run the docs, ex. waiting for the next release or the current state? But that's less of an issue, either way we get hosted docs 😎 .
My pleasure, hopefully this will help newcomers (including me 😄) explore Langium, and also make linking to relevant functionality easier.
The workflow has both triggers for this reason precisely 😁. I think it makes sense to publish it right away for v3.2, and after that it should be automatically updated with each new release.
Yes we do! 😎 |
TypeDocs of 3.2.0 (ish) are now online at https://eclipse-langium.github.io/langium/ 🎉 Thanks a lot for your work @aabounegm! |
Adds automatic documentation generation using TypeDoc, which gets published to GitHub Pages on release.
I did not implement version management yet. Currently, only the latest version of the docs is available.
A preview of the website can be found here for now: https://aabounegm.github.io/langium/