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

add(ci): Check that dependencies have all been published to crates.io on release PRs #8992

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

arya2
Copy link
Contributor

@arya2 arya2 commented Nov 4, 2024

Motivation

We've mistakenly published a Github release that relies on crates from a git source thrice now, so we want to add a test that runs on release PRs to check that there are no git sources in the Cargo.lock file.

This is a draft PR because the test still needs to be added to CI and run for PRs that have the A-release label.

Solution

  • Adds a simple test that searches the Cargo.toml file for "source = "git+".

TODO:

  • Add a CI job for the test that runs on PRs with the A-release label

Tests

This was tested with the restore-internal-miner branch to ensure that it panics when there are dependencies being pulled in from git sources.

PR Author's Checklist

  • The PR name will make sense to users.
  • The solution is tested.
  • The PR has a priority label.

PR Reviewer's Checklist

  • The PR Author's checklist is complete.
  • The PR resolves the issue.

@arya2 arya2 added A-devops Area: Pipelines, CI/CD and Dockerfiles C-testing Category: These are tests P-Low ❄️ labels Nov 4, 2024
@arya2 arya2 self-assigned this Nov 4, 2024
@github-actions github-actions bot added the C-trivial Category: A trivial change that is not worth mentioning in the CHANGELOG label Nov 4, 2024
upbqdn
upbqdn previously approved these changes Nov 5, 2024
Copy link
Member

@upbqdn upbqdn left a comment

Choose a reason for hiding this comment

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

Nice one.

@mpguerra
Copy link
Contributor

mpguerra commented Nov 6, 2024

Do we have an issue for this one? It's ok if not, we don't need to create one

@mpguerra mpguerra linked an issue Nov 6, 2024 that may be closed by this pull request
@upbqdn
Copy link
Member

upbqdn commented Nov 6, 2024

We don't.

@oxarbitrage
Copy link
Contributor

I think we should create a tracker issue "make changes to the release process" and this can be part of it with also #8934 and others we might need to add.

@gustavovalverde
Copy link
Member

@mergify refresh

Copy link
Contributor

mergify bot commented Nov 14, 2024

refresh

✅ Pull request refreshed

@oxarbitrage oxarbitrage added the A-release Area: Zebra releases and release management label Nov 20, 2024
@oxarbitrage oxarbitrage marked this pull request as ready for review November 20, 2024 20:48
@oxarbitrage oxarbitrage requested review from a team as code owners November 20, 2024 20:48
@oxarbitrage oxarbitrage requested review from gustavovalverde and oxarbitrage and removed request for a team November 20, 2024 20:48
@oxarbitrage
Copy link
Contributor

This seems to be working, i added the release label to the PR to test it: https://github.com/ZcashFoundation/zebra/actions/runs/11940756889/job/33284430420?pr=8992#step:2:721

It can probably be improved, i added @gustavovalverde as the reviewer.

Copy link
Member

@gustavovalverde gustavovalverde left a comment

Choose a reason for hiding this comment

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

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-devops Area: Pipelines, CI/CD and Dockerfiles A-release Area: Zebra releases and release management C-testing Category: These are tests C-trivial Category: A trivial change that is not worth mentioning in the CHANGELOG P-Medium ⚡
Projects
None yet
Development

Successfully merging this pull request may close these issues.

v2.0.0-rc.0 branch CI failed: release in Release binaries
5 participants