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

ci: make workflows runnable ad-hoc #5056

Merged
merged 1 commit into from
Feb 5, 2025
Merged

Conversation

conorsch
Copy link
Contributor

@conorsch conorsch commented Feb 5, 2025

Describe your changes

We want to publish the protobuf changes in protocol/lqt_support (#5010) to unblock web integrations. In order to run the workflow ad-hoc on a branch, we need to permit workflow_dispatch runs. The workflow_call additions are not currently used, but have been in the past, and permit triggering via cross-repo API.

While authoring these changes, I noticed that the protobuf lint job is not running against feature branches, meaning the PRs into protocol/lqt_support have skipped that check. We've been careful in authoring the proto changes, but we should still verify that check passes on the feature branch, prior to publishing.

Issue ticket number and link

Refs #5010.

Checklist before requesting a review

  • If this code contains consensus-breaking changes, I have added the "consensus-breaking" label. Otherwise, I declare my belief that there are not consensus-breaking changes, for the following reason:

    no code changes, only affects CI. will result in new protobuf changes to published to the repo.

We want to publish the protobuf changes in `protocol/lqt_support` (#5010)
to unblock web integrations. In order to run the workflow ad-hoc on a branch,
we need to permit `workflow_dispatch` runs. The `workflow_call`
additions are not currently used, but have been in the past, and permit
triggering via cross-repo API.

Also updates the buf lint job to run against *all* PRs, not just those
against main. This effectively reverts
b3204b1, which is over a year old, and
predates the stabilization of the protos.
@conorsch conorsch added the A-CI/CD Relates to continuous integration & deployment of Penumbra label Feb 5, 2025
@conorsch conorsch requested a review from TalDerei February 5, 2025 18:27
Copy link
Collaborator

@TalDerei TalDerei left a comment

Choose a reason for hiding this comment

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

This is what I had in mind! would the protos be published under a dedicated label, ie. protocol/lqt-branch tag in the buf registry?

@conorsch
Copy link
Contributor Author

conorsch commented Feb 5, 2025

Sadly no, see #1667 for context. Will revisit that issue because it looks like upstream buf tooling may finally support meaningful tags.

@conorsch conorsch merged commit 95dae9a into main Feb 5, 2025
10 checks passed
@conorsch conorsch deleted the ci-make-actions-usable-ad-hoc branch February 5, 2025 18:44
@conorsch
Copy link
Contributor Author

conorsch commented Feb 5, 2025

We've been careful in authoring the proto changes, but we should still verify that check passes on the feature branch, prior to publishing.

Good news, the buf lint action passes on the latest LQT changes. To test, I branched from protocol/lqt_branch, rebased on main, and ran the workflow against that new branch. The rebasing was necessary, because the changes from this PR 5056 are not (yet) in the protocol/lqt_branch branch, so the workflow cannot be run ad-hoc there:

ci-buf-attempt

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-CI/CD Relates to continuous integration & deployment of Penumbra
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants