Thanks for the quick cleanup response. > I will add Go 1.9 to the travis file, but maybe we should just remove the note that Go 1.7+ is supported to reduce confusion.
+1 for removing 1.7+ since it seems like a promise we can't easily keep. Kevin Risden On Sun, Apr 22, 2018 at 8:42 PM, Francis Chuang <francischu...@apache.org> wrote: > Thanks for the review, Kevin! Please see my response in line. > > On 23/04/2018 11:18 AM, Kevin Risden wrote: > >> * Ran tests at git hash >> * Ran tests from tar.gz >> * Checked hashes and GPG key of tar.gz >> * Reviewed source for missing Apache headers >> * Reviewed history and documentation >> >> Few comments: >> * Dockerfile - think CMD ["python", "app.py"]" is not needed? line could >> be >> removed >> > - Done > >> * go_development.md - docker-compose up - should be "docker-compose up >> --build" otherwise might test against old version >> > - Done > >> * dsn.go line 191 - typo - principal >> > - Done > >> * README.md - dep ensure - Should this point to a specific release? ie: >> github.com/apache/calcite-avatica-go@3.0.0 >> > - Dep will search for the latest tag in the repository (in general, that's > what people want). It's possible to force a specific version using ` > github.com/apache/calcite-avatica-go@3.0.0` > <http://github.com/apache/calcite-avatica-go@3.0.0>, but I don't think we > need to specifically document that as users of dep will know. > >> * README.md - godoc - this links to master - didn't see a way to link to a >> specific version? >> > - Godoc only supports master. However, all go projects (including the > golang/go repository) have godoc pointing to master, I think it's best to > follow convention here. > >> * README.md - Go 1.7+ is supported - are there tests for this? .travis.yml >> might be a good place to test multiple go versions >> > - This is an interesting one. The Go team supports the current version and > the last version. As of today, that would be 1.10.x and 1.9.x. 1.7.x and > 1.8.x are considered obsolete. Most Go projects I use (kubernetes and > docker are using 1.9.x and will move to 1.10.x soon). In 1.9.x, type > aliases were added to the language, so that https://golang.org/pkg/context > / and https://godoc.org/golang.org/x/net/context are equivalent. This > allows us to the context library in stdlib without breaking dependencies > and code that uses golang.org/x/net/context. > > I will add Go 1.9 to the travis file, but maybe we should just remove the > note that Go 1.7+ is supported to reduce confusion. > >> * .gitignore - doesn't ignore golang/intellij files >> > - I've copied all IDE files from Calcite's gitignore into avatica-go's > gitignore. > >> * goreportcard - >> https://goreportcard.com/report/github.com/apache/calcite-avatica-go - >> lists some minor things >> > - These are some minor things we can refactor later :) > > None of the above are show stoppers for me. >> >> +1 (non-binding) >> >> Kevin Risden >> >> On Sun, Apr 22, 2018 at 7:05 PM, Francis Chuang <francischu...@apache.org >> > >> wrote: >> >> Hi all, >>> >>> I have created a release for Apache Calcite Avatica Go 3.0.0, release >>> candidate 1. >>> >>> The release notes are available here: https://github.com/apache/calc >>> ite-avatica-go/blob/master/site/go_history.md >>> >>> The commit to be voted on: http://git-wip-us.apache.org/r >>> epos/asf/calcite-avatica-go/commit/273c53f62206c90e6a4fbb05f >>> 4ac82b7f62cb9a7 >>> >>> The hash is 273c53f62206c90e6a4fbb05f4ac82b7f62cb9a7 >>> >>> The artifacts to be voted on are located here: >>> https://dist.apache.org/repos/dist/dev/calcite/apache-calcit >>> e-avatica-go-3.0.0-rc1/ >>> >>> The hashes of the artifacts are as follows: >>> >>> src.tar.gz.md5 00 D1 D0 92 07 C0 60 86 A2 C5 65 E3 B8 60 13 12 >>> src.tar.gz.sha256 6D303B02 442A3999 88DE9234 F0DF733D 49F69B95 35B0C5BC >>> 706F9A14 FAD6F33A >>> >>> Release artifacts are signed with the following key: >>> https://people.apache.org/keys/committer/francischuang.asc >>> >>> Instructions for running the test suite is located here: >>> https://github.com/apache/calcite-avatica-go/blob/master/ >>> site/go_development.md#testing >>> >>> Please vote on releasing this package as Apache Calcite Avatica Go 3.0.0. >>> >>> >>> The vote is open for the next 72 hours and passes if a majority of >>> at least three +1 PMC votes are cast. >>> >>> [ ] +1 Release this package as Apache Calcite Avatica Go 3.0.0 >>> [ ] 0 I don't feel strongly about it, but I'm okay with the release >>> [ ] -1 Do not release this package because... >>> >>> >>> Here is my vote: >>> >>> +1 (binding) >>> >>> Francis >>> >>> >>> >
