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

Adds support of Go 1.6+ and 1.7+. #74

Merged
merged 3 commits into from Mar 7, 2016

Conversation

Projects
None yet
2 participants
@laysakura
Contributor

laysakura commented Mar 7, 2016

@mattn
Go 1.6 supports vendoring by default, so GO15VENDOREXPERIMENT env should not be required to use vendor/ directory.

Previously, gom uses _vendor/ directory even with Go version is 1.6+.
See: http://akirachiku.com/2016/03/01/go16-development.html#vendoring (Japanese)

gom should use vendor/ directory with Go 1.6+ by default.
But when GO15VENDOREXPERIMENT=0 is explicitly set with Go <= 1.6 && >1.7 , gom should use _vendor/ (or GOM_VENDOR_NAME when it's set) because vendoring is not supported by Go itself.

Quote from https://golang.org/doc/go1.6#go_command :

Go 1.5 introduced experimental support for vendoring, enabled by setting the GO15VENDOREXPERIMENT environment variable to 1. Go 1.6 keeps the vendoring support, no longer considered experimental, and enables it by default. It can be disabled explicitly by setting the GO15VENDOREXPERIMENT environment variable to 0. Go 1.7 will remove support for the environment variable.

nakatani.sho
Adds support of Go 1.6+ and 1.7+.
Go 1.6 supports vendoring by default, so `GO15VENDOREXPERIMENT` env should not required to use `vendor/` directory.
@laysakura

This comment has been minimized.

Show comment
Hide comment
@laysakura

laysakura Mar 7, 2016

Contributor

I'm trying to find a solution to the test failure...
(correct way to get Go version in runtime)

Contributor

laysakura commented Mar 7, 2016

I'm trying to find a solution to the test failure...
(correct way to get Go version in runtime)

@mattn

This comment has been minimized.

Show comment
Hide comment
@mattn

mattn Mar 7, 2016

ありがとうございます。
結果を bool の変数でキャッシュして下さい。

mattn commented on main.go in e66f666 Mar 7, 2016

ありがとうございます。
結果を bool の変数でキャッシュして下さい。

This comment has been minimized.

Show comment
Hide comment
@laysakura

laysakura Mar 7, 2016

Owner

結果を bool の変数でキャッシュして下さい。

わかりました。

https://travis-ci.org/mattn/gom/builds/114175763 のテスト落ちの件で悩んでいるのですが、
Goにおいてセマンティックバージョンを取得する良い方法が見つかりませんでした。
(Goのソースツリーで grep '1\.6' などはした)
runtime.Version() は必ずしもGoのセマンティックバージョンを返すわけではなく、このCI環境の devel +6ed1038 Mon Mar 7 01:30:07 2016 +0000 ようにビルド情報を返すこともあるようです。
See: https://golang.org/pkg/runtime/#Version

そこで、 e66f666#diff-7ddfb3e035b42cd70649cc33393fe32cR61 において err != nil になった場合のケースのハンドリングで悩んでいるのですが、

  • このままpanicとし、リリースバージョンのGoのみをサポート対象とする
    • CIではGoのバージョンを固定し、リリースバージョンを使うようにする
  • 関数の返り値としてtrueを返す
  • 関数の返り値としてfalseを返す
  • 関数の返り値として os.Getenv("GO15VENDOREXPERIMENT") == "1" の結果を返す
  • その他

のいずれが良いでしょうか?
ご意見いただければと思います。

Owner

laysakura replied Mar 7, 2016

結果を bool の変数でキャッシュして下さい。

わかりました。

https://travis-ci.org/mattn/gom/builds/114175763 のテスト落ちの件で悩んでいるのですが、
Goにおいてセマンティックバージョンを取得する良い方法が見つかりませんでした。
(Goのソースツリーで grep '1\.6' などはした)
runtime.Version() は必ずしもGoのセマンティックバージョンを返すわけではなく、このCI環境の devel +6ed1038 Mon Mar 7 01:30:07 2016 +0000 ようにビルド情報を返すこともあるようです。
See: https://golang.org/pkg/runtime/#Version

そこで、 e66f666#diff-7ddfb3e035b42cd70649cc33393fe32cR61 において err != nil になった場合のケースのハンドリングで悩んでいるのですが、

  • このままpanicとし、リリースバージョンのGoのみをサポート対象とする
    • CIではGoのバージョンを固定し、リリースバージョンを使うようにする
  • 関数の返り値としてtrueを返す
  • 関数の返り値としてfalseを返す
  • 関数の返り値として os.Getenv("GO15VENDOREXPERIMENT") == "1" の結果を返す
  • その他

のいずれが良いでしょうか?
ご意見いただければと思います。

@mattn

This comment has been minimized.

Show comment
Hide comment
@mattn

mattn Mar 7, 2016

Owner

@laysakura これを使って貰っていいでしょうか? https://github.com/mattn/gover

Owner

mattn commented Mar 7, 2016

@laysakura これを使って貰っていいでしょうか? https://github.com/mattn/gover

@laysakura

This comment has been minimized.

Show comment
Hide comment
@laysakura

laysakura Mar 7, 2016

Contributor

早すぎる...w
了解です! 少々お待ちください。

Contributor

laysakura commented Mar 7, 2016

早すぎる...w
了解です! 少々お待ちください。

@mattn

This comment has been minimized.

Show comment
Hide comment
@mattn

mattn Mar 7, 2016

Owner

go1.4.0 でなく go1.4 を返しますが、問題ないでしょうか?無ければマージします。

Owner

mattn commented Mar 7, 2016

go1.4.0 でなく go1.4 を返しますが、問題ないでしょうか?無ければマージします。

@laysakura

This comment has been minimized.

Show comment
Hide comment
@laysakura

laysakura Mar 7, 2016

Contributor

@mattn

go1.4.0 でなく go1.4 を返しますが、問題ないでしょうか?

問題ないです。下記確認しています。

    ver14, _ := version.NewVersion("1.4")
    ver140, _ := version.NewVersion("1.4.0")
    ver150, _ := version.NewVersion("1.5.0")

    fmt.Println(ver140.Equal(ver14))  // => true
    fmt.Println(ver150.GreaterThan(ver14))  // => true
Contributor

laysakura commented Mar 7, 2016

@mattn

go1.4.0 でなく go1.4 を返しますが、問題ないでしょうか?

問題ないです。下記確認しています。

    ver14, _ := version.NewVersion("1.4")
    ver140, _ := version.NewVersion("1.4.0")
    ver150, _ := version.NewVersion("1.5.0")

    fmt.Println(ver140.Equal(ver14))  // => true
    fmt.Println(ver150.GreaterThan(ver14))  // => true

mattn added a commit that referenced this pull request Mar 7, 2016

@mattn mattn merged commit 880ce9d into mattn:master Mar 7, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@mattn

This comment has been minimized.

Show comment
Hide comment
@mattn

mattn Mar 7, 2016

Owner

Thank you

Owner

mattn commented Mar 7, 2016

Thank you

achiku added a commit to achiku/dotfiles that referenced this pull request Mar 8, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment