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

add travis build for go versions 1.7.x -> 1.10.x #98

Merged
merged 20 commits into from
Feb 26, 2018
Merged

add travis build for go versions 1.7.x -> 1.10.x #98

merged 20 commits into from
Feb 26, 2018

Conversation

chemidy
Copy link
Contributor

@chemidy chemidy commented Feb 17, 2018

to ensure compatibility with older go version, add travis build from go 1.6.x to 1.10.x

@hiranya911
Copy link
Contributor

Looks like some of our test dependencies don't work on older Go versions. If it's just the appengine package, we should remove that test dependency.

Copy link
Contributor

@avishalom avishalom left a comment

Choose a reason for hiding this comment

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

Thanks !!
Try changing the imports as in:
#87
in auth_std.go and in storage.go

@chemidy
Copy link
Contributor Author

chemidy commented Feb 18, 2018

internal/http_client.go:47: req.WithContext undefined (type *http.Request has no field or method WithContext)

WithContext is a go 1.7 feature

I used ctxhttp.Do which support multiple go version

@chemidy
Copy link
Contributor Author

chemidy commented Feb 18, 2018

Any idea for building issue with go get -t -v ./... with go 1.6 and 1.7

firebase.google.com/go/integration/auth go build firebase.google.com/go/integration/auth: no buildable Go source files in /home/travis/gopath/src/firebase.google.com/go/integration/auth firebase.google.com/go/integration/firestore go build firebase.google.com/go/integration/firestore: no buildable Go source files in /home/travis/gopath/src/firebase.google.com/go/integration/firestore firebase.google.com/go/integration/iid go build firebase.google.com/go/integration/iid: no buildable Go source files in /home/travis/gopath/src/firebase.google.com/go/integration/iid firebase.google.com/go/integration/messaging go build firebase.google.com/go/integration/messaging: no buildable Go source files in /home/travis/gopath/src/firebase.google.com/go/integration/messaging firebase.google.com/go/integration/storage go build firebase.google.com/go/integration/storage: no buildable Go source files in /home/travis/gopath/src/firebase.google.com/go/integration/storage

I used a specific install on travis.yaml
- go get -t -v $(go list ./... | grep -v integration)

@chemidy
Copy link
Contributor Author

chemidy commented Feb 18, 2018

./firebase_test.go:465: t.Run undefined (type *testing.T has no field or method Run)

t.Run is a go 1.7 feature
For now, maybe we can put this 1.6 build in Allowed Failures section until we remove go 1.6 support ?

@avishalom avishalom changed the title add travis build for go version 1.6.x -> 1.10.x add travis build for go versions 1.6.x -> 1.10 Feb 18, 2018
Copy link
Contributor

@hiranya911 hiranya911 left a comment

Choose a reason for hiding this comment

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

LGTM. But at this point I think we should seriously consider dropping golang 1.6 support -- especially since some of our dependencies don't seem to support it. @chemidy @avishalom wdyt?

@@ -37,14 +38,22 @@ type HTTPClient struct {
ErrParser ErrorParser
}

// Request contains all the parameters required to construct an outgoing HTTP request.
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we leave this where it used to be?

@chemidy
Copy link
Contributor Author

chemidy commented Feb 20, 2018

Dropping golang 1.6 support is ok for me.

I think the only sticking point is on appengine with legacy api_version: go1.6, but for now is already broken due to req.WithContext not supported.

@hiranya911
Copy link
Contributor

@chemidy We are having an internal discussion on dropping golang 1.6 support. I will update this thread once a decision has been made.

@hiranya911
Copy link
Contributor

Ok, we are officially ending support for Golang 1.6 and earlier (in all honesty we never supported them to begin with, as discovered through this PR). @chemidy can you clean this PR up so that Travis builds will run against Golang 1.7 and above? We can revert any changes you had to do to compile against 1.6.

@chemidy chemidy changed the title add travis build for go versions 1.6.x -> 1.10 add travis build for go versions 1.7.x -> 1.10.x Feb 26, 2018
@chemidy
Copy link
Contributor Author

chemidy commented Feb 26, 2018

done

Copy link
Contributor

@hiranya911 hiranya911 left a comment

Choose a reason for hiding this comment

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

Thanks @chemidy. A couple of nits to fix, and then I can merge.

- go test -v -test.short ./...

- go test -v -race -test.short ./... # Run tests with the race detector.
- go vet -v ./... # Run Go static analyzer.
Copy link
Contributor

Choose a reason for hiding this comment

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

Add newline at end of file.

.travis.yml Outdated
before_install:
- go get github.com/golang/lint/golint
- go get github.com/golang/lint/golint #Golint requires Go 1.6 or later.
Copy link
Contributor

Choose a reason for hiding this comment

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

Add space after # --> # Golint requires...

.travis.yml Outdated
- go get github.com/golang/lint/golint #Golint requires Go 1.6 or later.

install:
# prio to go 1.8 package with only test go get returns : "no buildable Go source files ..."
Copy link
Contributor

Choose a reason for hiding this comment

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

Lets change this as: # Prior to golang 1.8, this can trigger an error for packages containing only tests.

Copy link
Contributor

@hiranya911 hiranya911 left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@hiranya911 hiranya911 self-assigned this Feb 26, 2018
@hiranya911 hiranya911 merged commit 387fa39 into firebase:dev Feb 26, 2018
hiranya911 added a commit that referenced this pull request Feb 28, 2018
* Lint (#96)

* fix misspelling

* add check error

* missing copyright

* Doc (#97)

* update readme with Authentication Guide & Release Notes

* fix a misspelling : separately

* fix missing newline before package

* add Go Report Card + update doc

* add travis build for go versions 1.7.x -> 1.10.x (#98)

* add build for go version 1.6.x -> 1.10.x

* fix 1.10 version

* fix context to golang.org/x/net/context for go 1.6 compatibility

* add race detector + go vet on build + build without failure on go unstable

* add go16 et go17 file due to req.withcontext which is only go 1.7

* fix context package

* update go16.go to remove WithContext

* update bad import

* remove unused func

* finally use ctxhttp.Do with multiple build version

* ignore integration package for install

* fix go get command

* put go 1.6.X in allow_failures dur to test failure

* fix inversion of code

* remove go 1.6 support

* revert initial version with req.WithContext

* fix travis to support go 1.10.x

* nits

* Import context from standard package (#101)

* Import context from standard package.

* Firebase Database API (#92)

* Experimental RTDB code

* Added ref.Set()

* Added Push(), Update(), Remove() and tests

* Adding Transaction() support

* Fixed Transaction() API

* Code cleanup

* Implemented Query() API

* Added GetIfChanged() and integration tests

* More integration tests

* Updated unit test

* More integration tests

* Integration tests for queries

* Auth override support and more tests

* More test cases; AuthOverride support in App

* Implemented AuthOverride support; Added tests

* Implementing the new API

* More code cleanup

* Code clean up

* Refactored the http client code

* More tests

* Boosted test coverage to 97%

* Better error messages in tests; Added license headers

* Added documentatioon and cleaned up tests

* Fixing a build break

* Finishing up documentation

* More test cases

* Implemented a reusable HTTP client API

* Added test cases

* Comment clean up

* Using the shared http client API

* Simplified the usage by adding HTTPClient

* using the new client API

* Using the old ctx import

* Using the old context import

* Refactored db code

* More refactoring

* Support for arbitrary entity types in the request

* Renamed fields; Added documentation

* Removing a redundant else case

* Code readability improvements

* Cleaned up the RTDB HTTP client code

* Added shallow reads support; Added the new txn API

* Implementing GetOrdered() for queries

* Adding more sorting tests

* Added Query ordering tests

* Fixing some lint errors and compilation errors

* Removing unused function

* Cleaned up unit tests for db

* Updated query impl and tests

* Added integration tests for ordered queries

* Removed With*() from query functions

* Updated change log; Added more tests

* Support for database url in auto init

* Support for loading auth overrides from env

* Removed db.AuthOverride type

* Renamed ao to authOverride everywhere; Other code review nits

* Introducing the QueryNode interface to handle ordered query results (#100)

* Database Sample Snippets (#102)

* Adding database snippets

* Adding query snippets

* Added complex query samples

* Updated variable name

* Fixing a typo

* Fixing query example

* Updated DB snippets to use GetOrdered()

* Removing unnecessary placeholders in Fatalln() calls

* Removing unnecessary placeholders in Fatalln() calls

* Handling FCM canonical error codes (#103)

* Formatting test file with gofmt (#104)

* Bumped version to 2.6.0 (#105)
hiranya911 pushed a commit that referenced this pull request Mar 15, 2018
* Renamed some tests and test parameters for clarity, and adhere to Go conventions (#74)

* clean unused types (#76)

* Create CHANGELOG.md (#75) (#79)

* Create CHANGELOG.md

Initial changelog based on https://firebase.google.com/support/release-notes/admin/go

* change instance ID format (#82)

Changing the format of the "non-existing" instance ID in the integration tests to comply with the expected iid format.

* Import context from golang.org/x/net/ for 1.6 compatibility (#87)

* import golang.org/x/net/context instead of context for 1.6 compatibility

* Document non existing name in integration tests for iid (#85)

* Revoke Tokens (#77)

Adding TokensValidAfterMillis property, RevokeRefreshTokens(), and VerifyIDTokenAndCheckRevoked().

* Firebase Cloud Messaging API (#81)

* Adding Firebase Cloud Messaging (#62)

* initial commit for adding Firebase Cloud Messaging

* add validator

* use http const in messaging test

* add client version header for stats

* init integration test

* add integration test (validated on IOS today)

* add comment with URL to enable Firebase Cloud Messaging API

* fix broken test

* add integration tests

* accept a Message instead of RequestMessage + and rename method + send  / sendDryRun

* update fcm url

* rollback url endpoint

* fix http constants, change responseMessage visibility, change map[string]interface{} as map[string]string

* fix http constants

* fix integration tests

* fix APNS naming

* add validators

* Added APNS types; Updated tests

* Added more tests; Fixed APNS serialization

* Updated documentation

* Improved error handling inFCM

* Added utils file

* Updated integration tests

* Implemented topic management operations

* Added integration tests

* Updated CHANGELOG

* Addressing code review comments

* Supporting 0 valued Aps.Badge

* Addressing some review comments

* Removed some unused vars

* Accepting prefixed topic names (#84)

* Accepting prefixed topic named

* Added a comment

* Using new FCM error codes (#89)

* Bumped version to 2.5.0 (#90)

* Lint (#96)

* fix misspelling

* add check error

* missing copyright

* Doc (#97)

* update readme with Authentication Guide & Release Notes

* fix a misspelling : separately

* fix missing newline before package

* add Go Report Card + update doc

* add travis build for go versions 1.7.x -> 1.10.x (#98)

* add build for go version 1.6.x -> 1.10.x

* fix 1.10 version

* fix context to golang.org/x/net/context for go 1.6 compatibility

* add race detector + go vet on build + build without failure on go unstable

* add go16 et go17 file due to req.withcontext which is only go 1.7

* fix context package

* update go16.go to remove WithContext

* update bad import

* remove unused func

* finally use ctxhttp.Do with multiple build version

* ignore integration package for install

* fix go get command

* put go 1.6.X in allow_failures dur to test failure

* fix inversion of code

* remove go 1.6 support

* revert initial version with req.WithContext

* fix travis to support go 1.10.x

* nits

* Import context from standard package (#101)

* Import context from standard package.

* Firebase Database API (#92)

* Experimental RTDB code

* Added ref.Set()

* Added Push(), Update(), Remove() and tests

* Adding Transaction() support

* Fixed Transaction() API

* Code cleanup

* Implemented Query() API

* Added GetIfChanged() and integration tests

* More integration tests

* Updated unit test

* More integration tests

* Integration tests for queries

* Auth override support and more tests

* More test cases; AuthOverride support in App

* Implemented AuthOverride support; Added tests

* Implementing the new API

* More code cleanup

* Code clean up

* Refactored the http client code

* More tests

* Boosted test coverage to 97%

* Better error messages in tests; Added license headers

* Added documentatioon and cleaned up tests

* Fixing a build break

* Finishing up documentation

* More test cases

* Implemented a reusable HTTP client API

* Added test cases

* Comment clean up

* Using the shared http client API

* Simplified the usage by adding HTTPClient

* using the new client API

* Using the old ctx import

* Using the old context import

* Refactored db code

* More refactoring

* Support for arbitrary entity types in the request

* Renamed fields; Added documentation

* Removing a redundant else case

* Code readability improvements

* Cleaned up the RTDB HTTP client code

* Added shallow reads support; Added the new txn API

* Implementing GetOrdered() for queries

* Adding more sorting tests

* Added Query ordering tests

* Fixing some lint errors and compilation errors

* Removing unused function

* Cleaned up unit tests for db

* Updated query impl and tests

* Added integration tests for ordered queries

* Removed With*() from query functions

* Updated change log; Added more tests

* Support for database url in auto init

* Support for loading auth overrides from env

* Removed db.AuthOverride type

* Renamed ao to authOverride everywhere; Other code review nits

* Introducing the QueryNode interface to handle ordered query results (#100)

* Database Sample Snippets (#102)

* Adding database snippets

* Adding query snippets

* Added complex query samples

* Updated variable name

* Fixing a typo

* Fixing query example

* Updated DB snippets to use GetOrdered()

* Removing unnecessary placeholders in Fatalln() calls

* Removing unnecessary placeholders in Fatalln() calls

* Handling FCM canonical error codes (#103)

* Formatting test file with gofmt (#104)

* Bumped version to 2.6.0 (#105)

* Formatting (simplification) changes (#107)

* Checking for unformatted files in CI (#108)

* Checking for unformatted files in CI

* Adding newline at eof

* Document Minimum Go Version (#111)

* Fix invalid endpoint URL for topic unsubscribe (#114)

* Fix error message for missing user (#113)

* Update CHANGELOG.md (#117)

* Removing unused member from auth.Client (#118)

* Support Go 1.6 (#120)

* all: use golang.org/x/net/context

* internal: use ctxhttp to use /x/ context

The 1.6 Request type doesn't have WithContext.

* all: don't use subtests to keep 1.6 compatibility

* integration: use float64 for fields with exp value

Values like -7e+07 cannot be parsed into ints in Go 1.6. So, use
floats instead.

* integration/messaging: use t.Fatal not log.Fatal

* travis: add 1.6.x

* changelog: mention addition of 1.6 support

* readme: mention go version support

* Bumped version to 2.6.1 (#121)

* Changlog updates (#123)
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