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

ci: Check all crate documentation in addition to target-specific ndk #129

Merged
merged 2 commits into from
Mar 20, 2021

Conversation

MarijnS95
Copy link
Member

Following #106 (review)

Only ndk docs are built for all android targets but the rest of the documentation remains untested. Especially now that we're expanding to provide more complete docs for ndk-build and cargo-apk should everything be tested, to make sure that things like intra-doc links remain in working order.

TODO

The ndk docs build successfully here thanks to the test feature (enabled by --all-features); does it still make a lot of sense to build-test the documentation for every target individually?

@msiglreith
Copy link
Contributor

does it still make a lot of sense to build-test the documentation for every target individually?

I don't expect any target specific doc parts tbh. Doing it once for one would be fine as well imo.

@MarijnS95
Copy link
Member Author

@msiglreith I guess it's safe to remove then, and have the documentation depend on the test flag (and iirc a cfg(doc) feature is on its way, though it was disabled again on a repo I was contributing to due to it not being propagated to all crates properly).

Only `ndk` docs are built for all `android` targets but the rest of the
documentation remains untested. Especially now that we're expanding to
provide more complete docs for ndk-build and cargo-apk should everything
be tested, to make sure that things like intra-doc links remain in
working order.
The previous commit tests all documentation for all features and crates
(not just `ndk`). The NDK crate and its documentation build without
issues thanks to the guards introduced by the `"test"` feature. Since
the `ndk` docs do not depend on any target-specific flags (and `ndk-sys`
does not contain any documentation) is is fine to build-test it just
once for the host architecture.
@MarijnS95
Copy link
Member Author

@msiglreith Removed it :)

Btw we have a lot of build steps now that do not use --target but are repeatedly ran for every rust-target in the matrix, wasting a ton of time and resources. As soon as this is merged I want to separate those out into two jobs that can run in parallel: some that build/test the host tools and docs, while the other simply only installs cargo-apk (bit of overlap with the build in the first job) followed by just the target-specific ndk check and apk build.

@msiglreith msiglreith merged commit 006b740 into rust-mobile:master Mar 20, 2021
@MarijnS95 MarijnS95 deleted the check-docs branch March 20, 2021 12:30
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.

2 participants