search

stephenplusplus / google-auto-auth

Making it as easy as possible to authenticate a Google API request
MIT License
34 stars 9 forks source link

Update dependencies #34

Closed ace-n closed 6 years ago

ace-n commented 6 years ago

As title.

The primary motive is to to pull in a google-auth-library bugfix for Container Builder, which is integrated in the new release of google-auth-library.

Note: ideally, this would be a patch or minor version bump. If this change involves a major version bump, we'll have to update any dependents in order to fix the Container Builder issues.

stephenplusplus commented 6 years ago

Thanks! Published as 0.9.2

ofrobots commented 6 years ago

It is great that this landed, but there is no CI in this repo, and the changes over in google-auth-library have been quite substantial. It would be good to add a CI, or possibly move the functionality from here to google-auth-library itself. /cc @JustinBeckwith

stephenplusplus commented 6 years ago

I ran locally before publishing. I have an issue to open CI here: https://github.com/stephenplusplus/google-auto-auth/issues/32

But, generally agree: if google-auth-library can make auth easier, like this library, then we should blow this one up.

(A discussion on this should probably happen in a different issue, depending on the direction you want to take: "let's move functionality" vs "we need CI")

ofrobots commented 6 years ago

I think a CI should exist regardless. There is already an issue open for it: https://github.com/stephenplusplus/google-auto-auth/issues/32

stephenplusplus commented 6 years ago

@ace-n @ofrobots I had to revert. Would anyone be willing to send another PR, given the changes noted in #35? As I recall, I initially tested against the 1.0.0 pre-release, and things worked fine without changes. It would probably be quicker for someone more familiar with 1.2.0 to take a shot at this.

ace-n commented 6 years ago

That might be something for @ofrobots to look into - as I'm not very familiar with these libraries, and simply massaged them until npm test passed. (Another reason why we should have CI + solid testing, as @ofrobots mentioned above.)

stephenplusplus commented 6 years ago

We all want CI, we just need to get around to adding it :)

RE: solid testing, we could definitely use some real integration tests.