-
Notifications
You must be signed in to change notification settings - Fork 2.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
Implement support for action workflows #1411
Conversation
All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter. We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the ℹ️ Googlers: Go here for more info. |
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.
Thank you, @joshuabezaleel!
It's looking good... just a few tweaks and I think we can get a second LGTM on this.
Thank you very much for the really great review, @gmlewis 🙂 |
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.
Thank you, @joshuabezaleel!
LGTM.
Awaiting second LGTM before merging.
Note to self: Ideally, merge this after merging #1402.
@martinssipenko - if you are comfortable, please feel free to review this PR and then we can hopefully get it merged after you approve it and after we get #1402 merged. |
@googlebot I consent. |
CLAs look good, thanks! ℹ️ Googlers: Go here for more info. |
LGTM |
Thank you, @martinssipenko! Awaiting the merge of #1402 now before merging this one. |
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 the PR 👍
Thank you for the reviews, @martinssipenko @gauntface @gmlewis !! 🎉 🎉 🎉 !! |
Thanks, @gauntface. I'm going to hold off merging this one until #1402 gets another LGTM and merged... as this is based off of that one. |
github/actions_secrets.go
Outdated
// without revealing their encrypted values. | ||
// | ||
// GitHub API docs: https://developer.github.com/v3/actions/secrets/#list-secrets-for-a-repository | ||
func (s *ActionsService) ListSecrets(ctx context.Context, owner, repo string, opt *ListOptions) (*Secrets, *Response, error) { |
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.
When you get a chance, could you please change opt
to opts
to be consistent with #1417?
(Here and anywhere else you find it in this PR, please.)
Thank you, @joshuabezaleel! |
(You will find them in files that you didn't previously modify, by the way.... and you might need to perform a "git merge master" into your branch to see them.) |
I hope that should do the trick. cc: @gmlewis |
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.
Perfect. Thank you, @joshuabezaleel!
LGTM.
Merging.
This PR adds support for managing the most recent update of Action Workflows.
The branch was created from PR #1402 .
Relates to: #1399