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
* Dockerfile - think CMD ["python", "app.py"]" is not needed? line could
* go_development.md - docker-compose up - should be "docker-compose up
--build" otherwise might test against old version
* dsn.go line 191 - typo - principal
* README.md - dep ensure - Should this point to a specific release? ie:
- 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 `
<http://firstname.lastname@example.org>, 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
- 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
* goreportcard -
lists some minor things
- These are some minor things we can refactor later :)
None of the above are show stoppers for me.
On Sun, Apr 22, 2018 at 7:05 PM, Francis Chuang <francischuang@xxxxxxxxxx
I have created a release for Apache Calcite Avatica Go 3.0.0, release
The release notes are available here: https://github.com/apache/calc
The commit to be voted on: http://git-wip-us.apache.org/r
The hash is 273c53f62206c90e6a4fbb05f4ac82b7f62cb9a7
The artifacts to be voted on are located here:
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
Release artifacts are signed with the following key:
Instructions for running the test suite is located here:
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: