-
Notifications
You must be signed in to change notification settings - Fork 182
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 CI job to add docs to Github pages #160
Conversation
Pull Request Test Coverage Report for Build 14ee6e9f6f85d3667ed9023db482105ba3754950-PR-160
💛 - Coveralls |
Cool! Are the docs posted yet for preview? Why does it say "this check was skipped"? |
I forgot to mention that. The URL is: https://unicode-org.github.io/icu4x-docs/ It says the check was skipped b/c it only runs on |
Why does https://unicode-org.github.io/icu4x-docs/icu4x/ show all of our dependencies as crates? Is there a way to fix that? |
Looks like cargo is able to omit dependencies if specifically asked to do so: |
Yep, thanks for the link. The new commit adds the |
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.
Optional: I don't know when you decide to lump things into "build-test.yml"
and when you split things into new files like you did with "coverage.yml"
. Personally, I think docs deserve their own file.
I initially considered a separate workflow, but then I confirmed the fact that workflows cannot depend on each other. Only jobs within a workflow can depend on each other. And I don't want the docs to be generated if the code doesn't build / tests don't pass. And since formatting is quicker thing to check than doc generation but is required for all commits, I also made that a dependency of docs too so that we don't generate docs needlessly when formatting needs cleanup. (I guess docs are generated from parts of code that adhere to a certain format, so that docs generation job dep on formatting should make sense for that reason, too.) |
Closes #111