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

adding examples for app auth #2240

Merged
merged 4 commits into from
Jan 3, 2022
Merged

adding examples for app auth #2240

merged 4 commits into from
Jan 3, 2022

Conversation

patalwell
Copy link
Contributor

@gmlewis opened a seperate PR from master, have a headache from trying to revert fetches I had pulled. I'd have to cherry pick commits and would rather start fresh.

To recap, this is a simple change to README.md and an example for GitHub app authentication flow so folks can understand how to leverage installations and access tokens.

Copy link
Collaborator

@gmlewis gmlewis left a comment

Choose a reason for hiding this comment

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

Thank you, @patalwell !
Just a few minor tweaks, please.

example/newfilewithappauth/main.go Outdated Show resolved Hide resolved
example/newfilewithappauth/main.go Outdated Show resolved Hide resolved
@patalwell
Copy link
Contributor Author

@gmlewis good to go!

@gmlewis
Copy link
Collaborator

gmlewis commented Dec 27, 2021

I'm not seeing any changes since my last review... did you push them?

Also, did you run go test ./... and go mod tidy ?
It looks like your new dependency was not added to go.mod... so these commands should help:

  • go get github.com/bradleyfalzon/ghinstallation
  • go test ./...
  • go mod tidy

@codecov
Copy link

codecov bot commented Dec 29, 2021

Codecov Report

Merging #2240 (600c17c) into master (4d7a0b5) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #2240   +/-   ##
=======================================
  Coverage   97.80%   97.80%           
=======================================
  Files         113      113           
  Lines       10205    10205           
=======================================
  Hits         9981     9981           
  Misses        156      156           
  Partials       68       68           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4d7a0b5...600c17c. Read the comment docs.

@patalwell
Copy link
Contributor Author

@gmlewis oops, wrong branch. Pushed to the right one this time. Made the logging changes.

I didn't want to run mod tidy since we ran into issues last time with the go.mod file and go.sum files, BUT ran the commands at your discretion :)

Ran a test as well, I originally never ran one since I didn't have a test file. Also ran a linter! there was an error with one of the imports, so ran golangci-lint run --fix

go.mod Outdated Show resolved Hide resolved
Copy link
Collaborator

@gmlewis gmlewis left a comment

Choose a reason for hiding this comment

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

Thank you, @patalwell !
LGTM.

Awaiting second LGTM from any other contributor to this repo before merging.

@patalwell
Copy link
Contributor Author

Thank you, @patalwell ! LGTM.

Awaiting second LGTM from any other contributor to this repo before merging.

@wesleimp mind taking a look at these changes?

@patalwell
Copy link
Contributor Author

Happy New Year! @gmlewis

Hey so dont want to lose any momentum here, anyway we can ping another commiter for review?

@gmlewis
Copy link
Collaborator

gmlewis commented Jan 3, 2022

Happy New Year to you too, @patalwell !

Maybe @Parker77 has time to review.

Copy link

@Parker77 Parker77 left a comment

Choose a reason for hiding this comment

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

LGTM.

@gmlewis
Copy link
Collaborator

gmlewis commented Jan 3, 2022

Thank you, @Parker77 !
Merging.

@gmlewis gmlewis merged commit dd254a0 into google:master Jan 3, 2022
@patalwell patalwell deleted the auth-example branch January 3, 2022 23:26
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.

3 participants