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

Adds methods for actions secrets #1236

Merged
merged 11 commits into from May 29, 2020
Merged

Conversation

jylitalo
Copy link
Contributor

@jylitalo jylitalo commented Apr 8, 2020

Still missing all specs for testing these methods, so current status is "works for me".

@jylitalo jylitalo changed the title Adds methods for actions secrets (WIP) Adds methods for actions secrets Apr 8, 2020
Copy link

@indigok indigok left a comment

Choose a reason for hiding this comment

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

👋@jylitalo

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.

Comment on lines +13 to +15
def get_public_key(repo)
get "#{Repository.path repo}/actions/secrets/public-key"
Copy link

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:

Suggested change
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

Copy link
Contributor Author

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.

lib/octokit/client/actions_secrets.rb Outdated Show resolved Hide resolved
lib/octokit/client/actions_secrets.rb Outdated Show resolved Hide resolved
@jylitalo
Copy link
Contributor Author

On my local machines, this currently fails with:

Failures:

  1) Octokit::Client::ActionsSecrets with repository with a secret .create_or_update_secret updating existing secret returns 204
     Got 0 failures and 2 other errors:

     1.1) Failure/Error: raise error

          Octokit::Unauthorized:
            POST https://api.github.com/user/repos: 401 - Bad credentials // See: https://developer.github.com/v3

@jylitalo
Copy link
Contributor Author

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.

@pschaumburg
Copy link

This solves #1216 right?

@jylitalo
Copy link
Contributor Author

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/
But the secrets part covers pretty much all that I need to manage via API.

@indigok
Copy link

indigok commented May 18, 2020

@jylitalo apologies for the late reply. You're welcome to add to workflow.yaml in this PR! And thank you for all the work you've done already 🙇🏻‍♀️

indigok pushed a commit that referenced this pull request May 28, 2020
@indigok
Copy link

indigok commented May 28, 2020

👋🏻 @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 ✨

@jylitalo
Copy link
Contributor Author

Rebased from 4-stable.

@jylitalo
Copy link
Contributor Author

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.

@indigok indigok merged commit 631d981 into octokit:4-stable May 29, 2020
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.

None yet

3 participants