-
Notifications
You must be signed in to change notification settings - Fork 6.8k
Add dependabot license fixup script #11269
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR introduces a script to automatically fix license-related failures in dependabot PRs by regenerating license files and pushing the fix commits back to the PRs. The script identifies open dependabot PRs with failing license checks, processes each one by running make licenses
, and commits/pushes the updated license files.
Key changes:
- Automated script to handle license failures in dependabot PRs
- Makefile target for convenient script execution
- Support for processing multiple PRs in batch
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
File | Description |
---|---|
script/fix-dependabot-licenses.sh | Main automation script that identifies failing PRs, checks out branches, regenerates licenses, and pushes fixes |
Makefile | Adds fix-dependabot-licenses target to invoke the new script |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! although I haven't tried it, that's why I'm just commenting.
.PHONY: fix-dependabot-licenses | ||
fix-dependabot-licenses: | ||
./script/fix-dependabot-licenses.sh |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about adding a workflow to kick this off every day? Or even triggered by new dependabot PR creation?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See PR description:
Big picture, it would be nice if this happened automatically and @andyfeller has pointed out docs in the past that might enable this, but I just thought I'd close out the day with some vibes.
I'm not going to do this for this PR, but would welcome follow up work to reduce the manual toil. There were security concerns about doing it on dependabot PR creation, which is why I do not want to get into it now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that is doable, but definitely more challenging since Dependabot PRs trigger workflows as if the PR comes from a fork, so we would need to introduce a pull_request_target
workflow, and that adds security considerations & complexity.
IMO let's wait and see if this is painful to run ourselves, and then if we feel it's needed, write a workflow in a future PR.
Edit: sorry this was sent before I saw Will's comment above. We're saying the same thing 🙂
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment t 8000 o others. Learn more.
Approving this but I just want to note that I did not test this because I don't think these one-shot scripts are worth the effort me trying to create a situation with some dependabot PRs in a cli/cli fork, and because I'm under the impression that you tested this with these PRs already and it worked 😃
I did read through the code and it LGTM. I agree with the Copilot comment though, I think being specific is good practice. I added a comment that I think might be nice if someone bumps into this script later without context, that's there to accept if you agree with it 🙏
9876ee6
to
26d70bf
Compare
I believe I addressed copilot and @BagToad's comments with changes. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know you've stated we're free to take or leave this script and noted our previous conversation about having a GitHub Actions workflow conditionally triggered by pull requests opened by Dependabot.
Genuinely, I would value implementing that fix which we deferred until a pain point rather than implementing an additional manual fix for maintainers to run.
echo "📋 Fetching open dependabot PRs..." | ||
|
||
# Get all open PRs by dependabot | ||
dependabot_prs=$(gh pr list --author "app/dependabot" --state open --json number,title) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thought: normally I've seen bots referred to as dependabot[bot]
🤔
I'm going to close the PR then until that is prioritised. The script is here if people want it. |
Description
Relates to #11213 and #11047
Since we now accept minor dependabot version bumps, and since those come with license changes that result in our license check failing, I was motivated to create a quick script to resolve the 4 currently open dependabot PRs:
The script looks for all open PRs by dependabot with a failing job, where the job is failing due to a failure in license checks, then it checks out the PR, runs
make licenses
and pushes a commit back with the changes.Big picture, it would be nice if this happened automatically and @andyfeller has pointed out docs in the past that might enable this, but I just thought I'd close out the day with some vibes.
Run Logs