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

update ci workflow #28

Merged
merged 3 commits into from
Jan 23, 2023
Merged

update ci workflow #28

merged 3 commits into from
Jan 23, 2023

Conversation

kturney
Copy link
Contributor

@kturney kturney commented Jan 22, 2023

Summary

Revamps and fixes CI workflow.

Updates

  • migrated rust-toolchain to newer rust-toolchain.toml
  • utilized the rustup included in ubuntu-latest to install the toolchain specified in rust-toolchain.toml
  • based Rust CI setup on the rust-analyzer CI workflow
    • added caching
    • added faster build ordering
    • added separation of test build and test execution
    • disabled incremental builds in CI
  • used available actions to install additional languages instead of leaning on apt-get directly

Potential shortcomings

  • had to use newer Swift than readme says serge-reflection supports (5.7 instead of 5.3) due to gpg issues with older Swift versions
  • had to add an include for std::numeric_limits to fix cpp tests, matching fix found here. I'm not sure if that is considered a breaking change.

Misc

Warnings during tests indicate that [email protected] is no longer supported. We could use this opportunity to bump the minimum support versions of different supported runtimes.

@kturney kturney requested a review from ma2bd as a code owner January 22, 2023 03:01
@kturney kturney marked this pull request as draft January 22, 2023 03:01
@kturney kturney force-pushed the ci-updates branch 7 times, most recently from 445eeae to a06b96d Compare January 22, 2023 04:28
fixes error: no member named 'numeric_limits' in namespace 'std'
@kturney kturney marked this pull request as ready for review January 22, 2023 07:03
@ma2bd
Copy link
Contributor

ma2bd commented Jan 23, 2023

@kturney Thanks for the PR. This looks great! My only ask is that we keep using the pinned version of Rust (aka the one from rust-toolchain) in CI. Otherwise, I believe CI may break (typically clippy) whenever ubuntu-latest changes.

Copy link
Contributor

@ma2bd ma2bd left a comment

Choose a reason for hiding this comment

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

back to your queue

@kturney
Copy link
Contributor Author

kturney commented Jan 23, 2023

Hi @ma2bd, thanks for the quick review!
Since the rust-toolchain.toml is there, CI is currently pinned to using 1.60.0 for everything.
And it's my understanding that will only change if we update the toolchain in the rust-toolchain.toml.
Is that not correct?

Are you wanting me to change every use of cargo to cargo +1.60.0 to be extra certain?

Copy link
Contributor

@ma2bd ma2bd left a comment

Choose a reason for hiding this comment

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

Oh I see. It looks like that rustup show actually installs the missing toolchain so we're good!

@ma2bd ma2bd merged commit 7d89de9 into zefchain:main Jan 23, 2023
@kturney kturney deleted the ci-updates branch January 23, 2023 05:03
@ma2bd ma2bd mentioned this pull request Jan 23, 2023
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