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

change(ci): When building crates individually, build all targets, and run clippy #8024

Merged
merged 7 commits into from
Nov 30, 2023

Conversation

teor2345
Copy link
Contributor

@teor2345 teor2345 commented Nov 28, 2023

Motivation

There are some build failures that only show up when the tests are built in an individual crate. This impacts Zebra's usability for external developers.

This PR runs clippy and builds all targets in all crates individually. Then it fixes any errors that turn up.

This is related to #8019 and the scanner work in general, but these errors could happen in any crate.

PR Author Checklist

Check before marking the PR as ready for review:

  • Will the PR name make sense to users?
  • Does the PR have a priority label?
  • Have you added or updated tests?
  • Is the documentation up to date?

If a checkbox isn't relevant to the PR, mark it as done.

Solution

  • Build all targets for individual crates, not just crates/binaries
  • Run clippy on all targets for individual crates
  • Use a medium runner because the zebrad test build takes more than 14 GB disk space
  • Fix the build bugs that shows up:
    • Activate required features in test dependencies
    • Remove redundant test-only features in prod dependencies

Related cleanups:

  • Standardise dependency order
  • Remove unnecessary async in test functions

Admin:

  • Add a branch protection rule for building zebra-scan individually

Testing

This PR fixes and adds tests.

Review

This is urgent because it will cause merge conflicts with upcoming changes.

Reviewer Checklist

Check before approving the PR:

  • Does the PR scope match the ticket?
  • Are there enough tests to make sure it works? Do the tests cover the PR motivation?
  • Are all the PR blockers dealt with?
    PR blockers can be dealt with in new tickets or PRs.

And check the PR Author checklist is complete.

@teor2345 teor2345 added C-bug Category: This is a bug A-devops Area: Pipelines, CI/CD and Dockerfiles P-High 🔥 I-build-fail Zebra fails to build C-trivial Category: A trivial change that is not worth mentioning in the CHANGELOG A-blockchain-scanner Area: Blockchain scanner of shielded transactions labels Nov 28, 2023
@teor2345 teor2345 self-assigned this Nov 28, 2023
@teor2345 teor2345 changed the title change(ci): Run clippy and build all targets on all crates individually change(ci): When building crates individually, build all targets, and run clippy Nov 29, 2023
@teor2345 teor2345 marked this pull request as ready for review November 29, 2023 03:38
@teor2345 teor2345 requested review from a team as code owners November 29, 2023 03:38
@teor2345 teor2345 requested review from oxarbitrage and removed request for a team November 29, 2023 03:38
Copy link
Contributor

@oxarbitrage oxarbitrage left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

mergify bot added a commit that referenced this pull request Nov 29, 2023
@mergify mergify bot merged commit cb9452c into main Nov 30, 2023
182 checks passed
@mergify mergify bot deleted the tests-indiv branch November 30, 2023 00:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-blockchain-scanner Area: Blockchain scanner of shielded transactions A-devops Area: Pipelines, CI/CD and Dockerfiles C-bug Category: This is a bug C-trivial Category: A trivial change that is not worth mentioning in the CHANGELOG I-build-fail Zebra fails to build
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants