Skip to content

Conversation

@jdsalaro
Copy link
Contributor

@jdsalaro jdsalaro commented Jun 20, 2025

We recently needed to change the AUTHOR_NAME and AUTHOR_EMAIL with which an internal bot triggers a PR to our BCR fork and therefore the commit that eventually lands to https://github.com/bazelbuild/bazel-central-registry

It probably makes sense to make these two inputs instead of hardcoded.

@aspect-workflows
Copy link

aspect-workflows bot commented Jun 20, 2025

Test

All tests were cache hits

12 tests (100.0%) were fully cached saving 1m 31s.


Buildifier      Format      Lint [.]

@jdsalaro jdsalaro marked this pull request as ready for review June 20, 2025 13:06
Copy link
Collaborator

@kormide kormide left a comment

Choose a reason for hiding this comment

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

Thanks! Looks good, just a few nits for the descriptions.

@kormide
Copy link
Collaborator

kormide commented Jun 22, 2025

Also, could you follow the pattern of putting the inputs into env vars as was done here? I think it's to satisfy some security tooling.

@jdsalaro jdsalaro requested review from Yannic and kormide June 23, 2025 10:20
@Yannic
Copy link

Yannic commented Jun 24, 2025

CI is read in this PR because the PR comes from a fork. #280 pushed the same changes to a branch in this repo and CI is happy

Copy link
Collaborator

@kormide kormide left a comment

Choose a reason for hiding this comment

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

LGTM. I have some manual tests that I run for the workflow because it doesn't have integration testing yet. I'll run those today then merge.

@kormide
Copy link
Collaborator

kormide commented Jun 24, 2025

Tested here with defaults and here with an overridden committer/author. Looks good.

@Yannic
Copy link

Yannic commented Jun 24, 2025

@jdsalaro is in Amsterdam, so probably done for today. Are you ok with me merging the PR after Jayson updated the comment @kormide?

@kormide
Copy link
Collaborator

kormide commented Jun 24, 2025

@jdsalaro is in Amsterdam, so probably done for today. Are you ok with me merging the PR after Jayson updated the comment @kormide?

Yeah you can merge and push a 0.2.2 tag if you have the permissions.

@jdsalaro
Copy link
Contributor Author

@Yannic Yannic changed the title Proposal: Add author_{name,email} and committer_{name,email} Inputs Add author_{name,email} and committer_{name,email} Inputs Jun 25, 2025
@Yannic
Copy link

Yannic commented Jun 25, 2025

I took the liberty to remove Proposal: from the title before merging. It's no longer just a proposal

@Yannic Yannic merged commit 096e472 into bazel-contrib:main Jun 25, 2025
7 of 9 checks passed
@Yannic
Copy link

Yannic commented Jun 25, 2025

Pushed v0.2.2

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants