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.

Sign up for GitHub

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

Jump to bottom

feat(CI): add external type check #2957

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

feat(CI): add external type check #2957

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
76ebc78
feat(CI): add external type check
ethanburrell Aug 23, 2022
02075f4
Merge branch 'master' of https://github.com/hyperium/hyper into add-c…
ethanburrell Aug 23, 2022
0cc10bf
install dependency in CI
ethanburrell Dec 5, 2022
df547c3
Merge branch 'master' into add-ci-check-external-types
ethanburrell Dec 5, 2022
60106ab
fix check-external-types install
ethanburrell Dec 5, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 20 additions & 0 deletions .github/workflows/CI.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -260,3 +260,23 @@ jobs:
with:
command: rustdoc
args: --features full,ffi -- --cfg docsrs --cfg hyper_unstable_ffi -D broken-intra-doc-links

check-external-types:
name: Check exposed types
needs: [style, test]
runs-on: ubuntu-latest
steps:
- name: Checkout
uses: actions/checkout@v1
Copy link
Contributor

Choose a reason for hiding this comment

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

We should use the latest version of this action. Older versions of this action depend on node versions that are deprecated in Github's runners.

Suggested change
uses: actions/checkout@v1
uses: actions/checkout@v3


- name: Install Rust
uses: actions-rs/toolchain@v1
Copy link
Contributor

Choose a reason for hiding this comment

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

This action is unmaintained. It seems like people are coalescing around dtolnay/rust-toolchain as an alternative solution.

with:
profile: minimal
toolchain: nightly
Copy link
Contributor

Choose a reason for hiding this comment

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

We should pin to a specific combination of nightly version/cargo-check-external-types that's known to work. The newest nightly version might introduce a new version of rustdoc JSON output that's incompatible with the version expected by cargo-check-external-type.

It'd be nicer if we could install the toolchain version listed in their toolchain file, but that's probably not necessary to get this PR in.

override: true

- name: cargo check-external-types
uses: actions-rs/cargo@v1
Copy link
Contributor

Choose a reason for hiding this comment

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

Same issue as the other actions-rs/* actions- I think here we can just invoke cargo manually.

with:
command: check-external-types
1 change: 1 addition & 0 deletions Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ libc = { version = "0.2", optional = true }
socket2 = { version = "0.4", optional = true }

[dev-dependencies]
cargo-check-external-types = "0.1.2"
Copy link
Member

Choose a reason for hiding this comment

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

Would it be possible to make the dependency only installed in the CI workflow? So that most local dev doesn't need to download the extra dep?

futures-util = { version = "0.3", default-features = false, features = ["alloc"] }
matches = "0.1"
num_cpus = "1.0"
Expand Down