-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Adds methods for actions secrets #1236
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.
Thanks for opening this up, we're excited to see this being added. Below are some review changes we'd appreciate seeing that would make it more consistent with the rest of the library. To get this merged, we'll also want to see the specs written! Let us know if you have any questions.
def get_public_key(repo) | ||
get "#{Repository.path repo}/actions/secrets/public-key" |
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.
A couple of really helpful things on all the methods would be adding the developer docs link and an options hash, so it would look something like this:
def get_public_key(repo) | |
get "#{Repository.path repo}/actions/secrets/public-key" | |
# @see https://developer.github.com/v3/actions/secrets/#get-your-public-key | |
def get_public_key(repo, options) | |
get "#{Repository.path repo}/actions/secrets/public-key", options |
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.
Created cdd9c74 based on this comment.
On my local machines, this currently fails with:
|
According to https://github.com/RubyCrypto/rbnacl/wiki/Installing-libsodium#windows we would need some windows specific stuff into workflow.yaml, so that we get libsolidium.dll into test environment. |
This solves #1216 right? |
I would say that it is only part of the solution, then there are at least artifacts and workflow API calls, which need there own implementations. @see https://developer.github.com/v3/actions/ |
@jylitalo apologies for the late reply. You're welcome to add to |
👋🏻 @jylitalo I'm getting access denied when I try to push to this branch, but if you can update it with 4-stable we should be good to merge. We've since removed the dependency on Windows builds ✨ |
This one is based on @hmharvey's suggestion.
Co-Authored-By: Heather Harvey <hmharvey@github.com>
Co-Authored-By: Heather Harvey <hmharvey@github.com>
Rebased from 4-stable. |
https://github.com/Yleisradio/octokit.rb/actions/runs/119094250 already reports that it passes tests, but for some reason it doesn't yet appear here. |
Still missing all specs for testing these methods, so current status is "works for me".