Skip to content
This repository has been archived by the owner on May 11, 2021. It is now read-only.

update Go version from 1.9 to 1.12 #33

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

mom0tomo
Copy link

@mom0tomo mom0tomo commented Aug 16, 2019

教材slackbotで使っているGAE Goのバージョンを1.12にアップデートし、go modulesを使ってパッケージの依存管理をする様に変更しました。

@sinmetal
Copy link

Go 1.12はApp Engine SDKから脱却した世界なので、これだけでは動かないと思う。

runtime:go111 であれば、これでいけます。

api_version: go1.9
module: slackbot
runtime: go112
module: default

Choose a reason for hiding this comment

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

moduleをdefaultに変えてしまうと、Deploy時に他のハンズオンのやつとぶつかるので、ちょっとあれかも?

Copy link
Author

Choose a reason for hiding this comment

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

なるほど!このような場合にどのようにmoduleに名前をつけていいのかわからず、エラーメッセージの通りにdefaultに変更していました。
https://cloud.google.com/appengine/docs/standard/go112/config/appref
このドキュメントを読んだのですが、moduleについて書かれておらず、そもそもGo1.12ではmoduleの定義をyamlから外した方がいいのでしょうか?

Copy link

@sonatard sonatard Aug 16, 2019

Choose a reason for hiding this comment

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

moduleはデプロイ先なので指定しないとdefaultにデプロイされてしまいます!
現在はmoduleはserviceに変わったので、以下のように設定するのが正しいと思います

service: slackbot

w.WriteHeader(http.StatusInternalServerError)
}
return
default:
log.Errorf(ctx, "不正なアクション: %s", action)
log.Printf("不正なアクション: %s", action)
Copy link

Choose a reason for hiding this comment

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

[IMO] ここだけログに出すメッセージが日本語なのが気になりました… invalid action とかのほうがいいかもしれません。

Copy link
Author

Choose a reason for hiding this comment

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

@tenntenn こちらは英語メッセージにしたほうがよいでしょうか?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants