-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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 usable auth, taking a dependency on the google-auth-library repo. #374
Conversation
A bunch of newlines at the ends of files are missing, please fix those. Also, what's the strategy on testing these changes? Seems a lot of tests were removed and I'm assuming moved to the other library we now depend on, however we should probably test that certain objects and functions are still exposed to the user and that all available auth objects still exist as they should. Any thoughts on that? |
I added newlines to all the files where they were missing. Good question about testing. There are still some auth tests within the test.oauth2.cs file. For example the 'should refresh if have refresh token but no access token' test spins up a Drive API, sets a refresh token, and makes sure that the auth library kicks in and fetches a refresh token before executing the API. |
Yeah, I noticed you didn't rip them all out which is good, and obviously it doesn't make sense to have them all stay. As suggested, it might be just good to ensure that all the objects we expect to be there are there, anything we have documented in our README (JWT, OAuth2, etc) should likely be tested to exist. Might also be good to recruit someone working on auth stuff to code review this thoroughly or at least give it a thumbs up and I'll merge. I'm maintaining this repo as a previous intern / future full-timer in my spare time :) so I'm unsure what the timeline for these changes are. Overall looks really good (I like all the deletions), though I haven't had a lot of time to go deep down into the auth repo. |
I added some new tests to ensure the existence of the jwt and compute types. The other types are covered already. Thanks. |
Please re-sync the PR as at the moment, it can't be merged. |
Missing: package.json should be updated
|
…to the google-auth-library-nodejs repo, where they now belong
Re-synced the PR, merged in the latest changes, merged in those changes to the google-nodejs-auth-library repo (where they now belong), removed the changes from this repo, updated the package version, and updated the list of maintainers. |
Adds usable auth, taking a dependency on the google-auth-library repo.
Woohoo! Congrats on shipping! :) Can you document the changes in the GitHub release? https://github.com/google/google-api-nodejs-client/releases/tag/v2.0.0 Also we have a migration guide from 0.x to 1.x so now documenting going to 2.x might be appropriate with the auth related changes that are needed. Either that or remove the old migration guide because it might be confusing to users now using the 2.x library. |
No description provided.