-
Notifications
You must be signed in to change notification settings - Fork 259
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
Introduce an option to use the GitHub API to commit changes, for GPG #391
base: main
Are you sure you want to change the base?
Conversation
🦋 Changeset detectedLatest commit: 893ba16 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
This is a clever solution but tbh... I'm not sure if I like it for this project. It feels less maintainable - with git itself we have certain freedom in how we interact with it. I'm also worried that this would result in hitting rate limits more often (which already is an issue that this action hits from time to time). |
Hmm rate limits is a fair concern. Would you support having this as an opt-in feature then rather than replacing the existing git functionality for those of us that require signed commits? 🤔 |
I don't like having options like this but then I also see the value in this approach 😅 cc @emmatown thoughts? |
I was looking into the rate-limit issue again, and I realize I actually experienced it here: https://github.com/s0/ghcommit/actions/runs/10548632928/job/29222763569 (and can see from #192 that it's the same rate-limit as I faced). My guess is that we can probably improve things a lot by avoiding the search APIs, which typically have higher rate-limit costs (at least 5x according to https://docs.github.com/en/rest/using-the-rest-api/rate-limits-for-the-rest-api?apiVersion=2022-11-28#calculating-points-for-the-secondary-rate-limit). But beyond that, primary rate limits for the search API are tracked separately to other REST endpoints, so it wouldn't surprise me if there's some conflict / interference with other tokens / repos based on e.g. which runners these are executed on? Given that, at least with the primary rate limits, the GitHub Actions I imagine that we'll be able to avoid hitting the limits at all if we switch from the search API to the pull request list API, which provides filters for everything you use the search API for: https://docs.github.com/en/rest/pulls/pulls?apiVersion=2022-11-28#list-pull-requests I could open a separate PR for this change if you like? |
PR opened :) #393 |
Hi @dalmena |
+1 |
Any more thoughts on this PR? I could update the PR to make this method opt-in or opt-out and allow the user to choose between the 2 methods? |
To allow for signed tags to be created, rather than use the git CLI to push tags, manually push each tag using the GitHub API, which will sign the tag using the built-in GitHub GPG key.
To allow for all commits to be signed, use the GitHub API to push changes.
a369cc5
to
50b4be2
Compare
Hmm looks like I've just started experiencing some errors with this test branch: https://github.com/arcanejs/arcanejs/actions/runs/11418428423 Going to convert this back to draft now until I can investigate. |
I could consider making this an opt-in - once you resolve the issues you found |
+1 for this feature. I would opt-in to it |
Issue resolved in s0/ghcommit#23, so latest update fixes things. Converting this to be opt-in now |
Change this to a minor version bump, with a new feature that allows for using the GitHub API to create tags and commits.
06a6067
to
893ba16
Compare
Okay comments addressed, this is now opt-in via the input |
Tested here:
|
Any other thoughts @Andarist? |
+1 for this feature 🤩 |
good day, if it concerns my smart contracts, I'm sorry for not answering, I
get 1000 emails from companies where I would like to ask if you could help
me with my smart contracts, thank you
út 10. 12. 2024 v 13:40 odesílatel Jan Jonas ***@***.***>
napsal:
… +1 for this feature 🤩
—
Reply to this email directly, view it on GitHub
<#391 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AYJT2HHVTRARDMH6YSJ526D2E3OKPAVCNFSM6AAAAABNCXETLOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDKMZRGUZDQMJZHA>
.
You are receiving this because you are subscribed to this thread.Message
ID: ***@***.***>
|
fixes #392
Turns out that there wasn't any good TypeScript libraries for making modifications to files directly, let alone one that looks at the files changed locally to determine what needs to be pushed to GitHub, so I went ahead and created one here: https://github.com/s0/ghcommit
I then used this repo as a test-case for the updated actions to see if it all works, and so far so good.
Tags are now signed by default:
And commits are also signed, and attributed to github-actions:
It also works with more complex situations, such as custom
version
commands, like in this repo where it auto-bumps the major version in the README: Version Packages s0/changesets-action#2Would you like this functionality? And if so, do we want to just switch to this behavior by default + have a major version bump? or do we want to hide it behind an action input / argument for now and have it as opt-in?
My thinking is that just switching to this behavior overall makes the most sense, as most people probably want to attribute the commits to the owner of the
GITHUB_TOKEN
, and more and more people are going to require that commits are signed as the industry takes supply-chain-security more and more seriously.