doghouse: sunset google.golang.org/appengine module based on #307 #312
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The appengine packages that were designed for the go1 runtime, were left functional in go111, but Google is requesting that users preemptively migrate away due to their rigid structure and incompatibility with other environments. This change affects the reviewdog module dependencies, but code changes are limited to doghouse. The appengine runtime has been increased to go112 for extended support and to enforce that appengine packages break if used. appengine/urlfetch.Client has been migrated to net/http.Client. appengine/datastore has been migrated to cloud.google.com/go/datastore. This took more work than the Client changes, since the two packages are incompatible. There may be work to further refactor the abstractions, but that is out of scope for this change. appengine.IsDevServer has no clear successor, thus its usage and conditional logic has been purged. If there is intereset to have a comprehensive local development version of doghouse, this can be added back, but you will lose all the other mocks that dev_appserver.py offers: user auth, datastore access, etc. appengine/log contains leveled print functions, Errorf/Infof/etc., that have no clear parallel in the stdlib log package. As a stopgap, [SEVERITY] prefixes have been added to all the new log.Printf calls. This is not indended as a comprehensive solution, and it may be better to move to another package for logging, say logrus or log15, but it is out of scope at the moment. Finally, a note: with the change from using appengine.Context to using http.Request.Context, there are places where an http.Request and context.Context are passed. This could be simplified, but is out of scope. Fixes #226
https://developer.github.com/v3/apps/#get-a-user-installation Fix: #152 http.RoundTripper arg of ghinstallation.NewAppsTransport and ghinstallation.New must not be nil.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Added following changes based on #307
urlfetch.Client(ctx)have Transport but defaulthttp.Client's Transport is nil and github.com/bradleyfalzon/ghinstallation failed with nil exception.