Skip to content

fix(build): add beta rust to CI#4637

Merged
gustavovalverde merged 5 commits intomainfrom
add-beta-rust
Jun 22, 2022
Merged

fix(build): add beta rust to CI#4637
gustavovalverde merged 5 commits intomainfrom
add-beta-rust

Conversation

@gustavovalverde
Copy link
Member

@gustavovalverde gustavovalverde commented Jun 17, 2022 •
edited by teor2345
Loading

Motivation

On 29 July 2021, Zebra's tests started failing on beta Rust (rust-lang/rust#87483). This Rust PR fixed the test build failure we were seeing: rust-lang/rust#87483 and it was also backported.

Fixes #2541

Depends-On: #4668

Solution

  • Re-enable the Rust beta tests in the CI matrix.

Review

Anyone from @ZcashFoundation/devops-reviewers

@gustavovalverde gustavovalverde added A-devops Area: Pipelines, CI/CD and Dockerfiles C-enhancement Category: This is an improvement P-Medium ⚡ I-build-fail Zebra fails to build labels Jun 17, 2022
@gustavovalverde gustavovalverde requested a review from a team as a code owner June 17, 2022 02:45
@gustavovalverde gustavovalverde self-assigned this Jun 17, 2022
@gustavovalverde gustavovalverde requested review from dconnolly and removed request for a team June 17, 2022 02:45
teor2345
teor2345 suggested changes Jun 17, 2022
View reviewed changes
Copy link
Contributor

@teor2345 teor2345 left a comment

Choose a reason for hiding this comment

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

I think running beta Rust on macOS will be slow, and it seems pretty redundant

teor2345
teor2345 reviewed Jun 17, 2022
View reviewed changes
Copy link
Contributor

@teor2345 teor2345 left a comment

Choose a reason for hiding this comment

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

Do we want to require the beta tests to pass before we merge PRs?

I think the answer is "yes, but we should disable them and open a ticket if they fail".

@gustavovalverde
Copy link
Member Author

gustavovalverde commented Jun 17, 2022

Do we want to require the beta tests to pass before we merge PRs?

Yes. This shouldn't hurt

teor2345
teor2345 previously requested changes Jun 20, 2022
View reviewed changes
Copy link
Contributor

@teor2345 teor2345 left a comment

Choose a reason for hiding this comment

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

Do we want to require the beta tests to pass before we merge PRs?

Yes. This shouldn't hurt

Can you please add patch jobs to this PR, so we can require these jobs in GitHub branch protection settings?

This was referenced Jun 20, 2022
Closed
Merged
@teor2345
Copy link
Contributor

teor2345 commented Jun 21, 2022

@Mergifyio update

@mergify
Copy link
Contributor

mergify bot commented Jun 21, 2022

update

✅ Branch has been successfully updated

@gustavovalverde gustavovalverde enabled auto-merge (squash) June 21, 2022 10:57
@gustavovalverde
Copy link
Member Author

gustavovalverde commented Jun 21, 2022

Can you please add patch jobs to this PR, so we can require these jobs in GitHub branch protection settings?

A patch already exists for this job, we're just adding extra arguments to it. This new arguments are already available as a required check.

teor2345
teor2345 reviewed Jun 21, 2022
View reviewed changes
@teor2345 teor2345 mentioned this pull request Jun 21, 2022
Merged
1 task
@teor2345
Copy link
Contributor

teor2345 commented Jun 21, 2022

Can you please add patch jobs to this PR, so we can require these jobs in GitHub branch protection settings?

A patch already exists for this job, we're just adding extra arguments to it. This new arguments are already available as a required check.

If we want to require that "Test beta on ubuntu-latest" passes, we need a patch job for it.

I opened PR #4668 to add these patch jobs, and fix up some we missed from PR #4640.
(In general, I'd like to keep patch jobs in sync with real jobs, so we can quickly require jobs if we need to.)

@teor2345 teor2345 dismissed their stale review June 21, 2022 23:33

The patch jobs are now in a separated PR that this PR Depends-On

@teor2345
Copy link
Contributor

teor2345 commented Jun 22, 2022

@Mergifyio update

@mergify
Copy link
Contributor

mergify bot commented Jun 22, 2022

update

✅ Branch has been successfully updated

teor2345
teor2345 approved these changes Jun 22, 2022
View reviewed changes
Copy link
Contributor

@teor2345 teor2345 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, I can do the branch protection rules after it merges

@gustavovalverde gustavovalverde merged commit d2d4f5a into main Jun 22, 2022
@gustavovalverde gustavovalverde deleted the add-beta-rust branch June 22, 2022 04:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@dconnolly dconnolly Awaiting requested review from dconnolly dconnolly was automatically assigned from ZcashFoundation/devops-reviewers

1 more reviewer

@teor2345 teor2345 teor2345 approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@gustavovalverde gustavovalverde

Labels

A-devops Area: Pipelines, CI/CD and Dockerfiles C-enhancement Category: This is an improvement I-build-fail Zebra fails to build

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Re-enable beta Rust tests in CI

2 participants

@gustavovalverde @teor2345