-
Notifications
You must be signed in to change notification settings - Fork 181
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
fix: Correct dependencies and build checks #484
Changes from 1 commit
a311c80
1a6466a
e13dae3
4142580
6514879
402c8b5
171bf1d
7b6289c
b6a803d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -135,7 +135,7 @@ jobs: | |
uses: actions-rs/cargo@v1 | ||
with: | ||
command: check | ||
args: --all --bins --tests --examples | ||
args: --workspace --all-features --bins --tests --examples --benches | ||
|
||
- name: tests | ||
uses: actions-rs/cargo@v1 | ||
|
@@ -145,17 +145,17 @@ jobs: | |
# GHA if this is not present | ||
# https://twitter.com/steipete/status/921066430220652544?lang=en | ||
# https://blog.phusion.nl/2017/10/13/why-ruby-app-servers-break-on-macos-high-sierra-and-what-can-be-done-about-it/ | ||
OBJC_DISABLE_INITIALIZE_FORK_SAFETY: YES | ||
OBJC_DISABLE_INITIALIZE_FORK_SAFETY: "YES" | ||
with: | ||
command: test | ||
args: --all -j 4 | ||
args: -j 4 --workspace --all-features --examples --benches --bins | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. is There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Heh, welcome to the world of CI runners :) If you want to help things along it might make sense to drop down to -j 2 for a moment. The reasoning is that most of these (esp. GHA runners) OOM very easily if you hit the tests/builds hard. Hence manually limiting to 4 and now potentially even lower for windows builds. This should all go away once we move to our infra. |
||
|
||
- name: clipy | ||
- name: clippy | ||
uses: actions-rs/cargo@v1 | ||
if: matrix.os == 'ubuntu-latest' && matrix.rust=='stable' | ||
with: | ||
command: clippy | ||
args: --all --tests --benches --all-targets -- -D warnings | ||
command: clippy | ||
args: --workspace --all-features --tests --benches --examples --bins --all-targets -- -D warnings | ||
|
||
- name: build release | ||
uses: actions-rs/cargo@v1 | ||
|
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.
why having
--all-features
is good to test against, we also need to have tests (or at least build checks) for each individual feature, to make sure they work by themselves and with no features enabled at allThere 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.
Yes, that's true. I was trying to take small steps.
Given how difficult it was to get this to be happy in CI I'd prefer to do that as a followup since it already fixes some things on main and would stop regressions to that instead of having to keep playing catchup with main while working on this - as already happened.
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.
this was not meant to add more work to this pr, just a reminder for all of us that it needs more work 😅
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.
Made an issue to not forget: https://github.com/n0-computer/iroh/issues/496