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

Upgrade Golang Version 1.15 → 1.18 #272

Merged
merged 2 commits into from
Jun 11, 2022

Conversation

panorama32
Copy link
Member

@panorama32 panorama32 commented Jun 4, 2022

Related Issue

What

上げようと思った要因
test_and_lint.yamlGet toolsで以下のようなエラーが出て、CIの結果が根こそぎ赤になっている。
(os.ReadDiros.ReadFileもadded in go1.16なのが原因)

Run GO111MODULE=off go get -u github.com/skeema/skeema
# github.com/skeema/skeema/vendor/github.com/containerd/containerd/sys
Error: ../../../go/src/github.com/skeema/skeema/vendor/github.com/containerd/containerd/sys/fds.go:30:15: undefined: os.ReadDir
Error: ../../../go/src/github.com/skeema/skeema/vendor/github.com/containerd/containerd/sys/oom_linux.go:[7](https://github.com/camphor-/relaym-server/runs/6196083637?check_suite_focus=true#step:7:8)1:15: undefined: os.ReadFile
Error: Process completed with exit code 2.

Memo

@panorama32 panorama32 requested a review from p1ass as a code owner June 4, 2022 07:04
@codecov
Copy link

codecov bot commented Jun 4, 2022

Codecov Report

Merging #272 (3674101) into master (e498ae9) will not change coverage.
The diff coverage is n/a.

❗ Current head 3674101 differs from pull request most recent head 043bc1a. Consider uploading reports for the commit 043bc1a to get more accurate results

@@           Coverage Diff           @@
##           master     #272   +/-   ##
=======================================
  Coverage   46.32%   46.32%           
=======================================
  Files          34       34           
  Lines        2081     2081           
=======================================
  Hits          964      964           
  Misses       1071     1071           
  Partials       46       46           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 545fdaf...043bc1a. Read the comment docs.

@panorama32 panorama32 changed the title Upgrade Golang Version in test_and_lint workflow Upgrade Golang Version 1.15 → 1.18 Jun 4, 2022
@sanposhiho
Copy link
Member

sanposhiho commented Jun 4, 2022

バージョンアップよりも、tools.go(blank import書いてツールをimportして、go.modでバージョン固定するやつ)でskeemaの使用バージョン固定したほうが良い気がする。(Go1.18にはいくつかのリンターが対応してなくて落ちる(もしくはそもそも実行されない)問題があるから、まだ上げたくない。)

@panorama32
Copy link
Member Author

バージョンアップよりも、tools.go(blank import書いてツールをimportして、go.modでバージョン固定するやつ)でskeemaの使用バージョン固定したほうが良い気がする。(Go1.18にはいくつかのリンターが対応してなくて落ちる(もしくはそもそも実行されない)問題があるから、まだ上げたくない。)

こんな感じでしょうか
https://github.com/camphor-/relaym-server/pull/273

@sanposhiho
Copy link
Member

良さそう!

@sanposhiho
Copy link
Member

sanposhiho commented Jun 5, 2022

今気がついたけど

(Go1.18にはいくつかのリンターが対応してなくて落ちる(もしくはそもそも実行されない)問題があるから、まだ上げたくない。)

って言ったけど、このプロジェクトgolangci-lint入ってなかったわ…

@panorama32
Copy link
Member Author


どっちが良いかな
(あるいはtoolsでコマンドラインツールを固定するのも、Goのバージョン上げるのも両方やった方が良い?)

@sanposhiho
Copy link
Member

👍
どっちにしろtoolsのバージョンは固定しておくべきなのでどっちもmergeすることにしよう。

@sanposhiho
Copy link
Member

先にtoolsの方mergeして、こっちのbranchをrebase後でgo mod tidy走らせ直した方がいいかな

go.mod Outdated
Comment on lines 11 to 12
github.com/gorilla/websocket v1.5.0
github.com/labstack/echo/v4 v4.7.2
Copy link
Member

Choose a reason for hiding this comment

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

なんでこの二つのversionも上がってる?(Go1.18対応してるかどうかの関係?

Copy link
Member Author

Choose a reason for hiding this comment

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

エディタのUpgrade transitive dependenciesという機能を押してしまったかもしれないです。
修正してrebaseしました。

@panorama32 panorama32 force-pushed the upgrade-golang-ver-in-workflow branch from f39def6 to 043bc1a Compare June 11, 2022 04:06
Copy link
Member

@sanposhiho sanposhiho left a comment

Choose a reason for hiding this comment

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

:yoshi:

@panorama32 panorama32 merged commit dd92d3a into master Jun 11, 2022
@panorama32 panorama32 deleted the upgrade-golang-ver-in-workflow branch June 11, 2022 05:36
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.

2 participants