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

fix(build): Stop using a test-only method in production code #4000

Merged
merged 3 commits into from
Mar 30, 2022
Merged

fix(build): Stop using a test-only method in production code #4000

merged 3 commits into from
Mar 30, 2022

Conversation

teor2345
Copy link
Contributor

Motivation

Zebra fails to build from the README instructions, because it calls some test-only code.

cargo allows this to happen because:

  1. CI always builds tests
  2. Some Zebra crates have missing feature dependencies

Solution

  • Fix feature dependencies within Zebra crates - this makes test and non-test builds fail
  • Use the production method - this makes all builds work

Review

@dconnolly can review this PR.

Reviewer Checklist

  • Build succeeds using --locked

Follow Up Work

@teor2345 teor2345 added C-bug Category: This is a bug A-dependencies Area: Dependency file updates A-rust Area: Updates to Rust code P-High 🔥 labels Mar 30, 2022
@teor2345 teor2345 self-assigned this Mar 30, 2022
@teor2345 teor2345 requested a review from a team as a code owner March 30, 2022 04:45
@teor2345 teor2345 requested review from conradoplg and dconnolly and removed request for a team and conradoplg March 30, 2022 04:45
@teor2345 teor2345 mentioned this pull request Mar 30, 2022
Merged
2 tasks
@teor2345 teor2345 changed the title fix(build): Use a production method fix(build): Stop using a test-only method in production code Mar 30, 2022
@dconnolly
Copy link
Contributor

Failed to find proptest_derive:
image

@teor2345
Copy link
Contributor Author

Failed to find proptest_derive: ...

Alright let's make it really consistent

conradoplg
conradoplg approved these changes Mar 30, 2022
View reviewed changes
Copy link
Collaborator

@conradoplg conradoplg left a comment

Choose a reason for hiding this comment

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

Looks good, tested it locally

mergify bot added a commit that referenced this pull request Mar 30, 2022
@mergify
Merge of #4000
1dcef12
@mergify mergify bot mentioned this pull request Mar 30, 2022
Closed
19 tasks
@mergify mergify bot merged commit d767f43 into main Mar 30, 2022
@mergify mergify bot deleted the fix-locked-build branch March 30, 2022 18:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@conradoplg conradoplg conradoplg approved these changes

@dconnolly dconnolly Awaiting requested review from dconnolly

Assignees

@teor2345 teor2345

Labels
A-dependencies Area: Dependency file updates A-rust Area: Updates to Rust code C-bug Category: This is a bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@teor2345 @dconnolly @conradoplg