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

use go 1.13 #8088

Merged
merged 13 commits into from Sep 12, 2019

Conversation

@techknowlogick
Copy link
Member

commented Sep 4, 2019

as title

@techknowlogick techknowlogick added this to the 1.10.0 milestone Sep 4, 2019

@sapk
Copy link
Member

left a comment

You need to update go.mod by upgrading the first line to enable 1.13 libs.

@GiteaBot GiteaBot added the lgtm/need 2 label Sep 4, 2019

@techknowlogick

This comment has been minimized.

Copy link
Member Author

commented Sep 5, 2019

@sapk seems golangci-lint fails, and the reports contain incorrect information. I will look into this more.

go.mod Show resolved Hide resolved
@lafriks

This comment has been minimized.

Copy link
Member

commented Sep 5, 2019

You need to update go.mod by upgrading the first line to enable 1.13 libs.

@sapk why? imho it uses std libs from what golang compiler is used not what is specified in go.mod

@sapk

This comment has been minimized.

Copy link
Member

commented Sep 5, 2019

@lafriks I need to find the link but I remember that the std lib are also versionned now and that go compiler will use the one specified in go.mod

@sapk

This comment has been minimized.

Copy link
Member

commented Sep 5, 2019

From https://golang.org/doc/go1.13

If your code uses modules and your go.mod files specifies a language version, be sure it is set to at least 1.13 to get access to these language changes. You can do this by editing the go.mod file directly, or you can run go mod edit -go=1.13.

But I have also seen that some suggest to declare the minimal go version supported. This seems logic since it should be the lowest required std libs. I don't know go internal to say what is the best.

On a sidenote, I hope we can use new error methods soon.

@lafriks

This comment has been minimized.

Copy link
Member

commented Sep 6, 2019

From https://golang.org/doc/go1.13

If your code uses modules and your go.mod files specifies a language version, be sure it is set to at least 1.13 to get access to these language changes. You can do this by editing the go.mod file directly, or you can run go mod edit -go=1.13.

But I have also seen that some suggest to declare the minimal go version supported. This seems logic since it should be the lowest required std libs. I don't know go internal to say what is the best.

On a sidenote, I hope we can use new error methods soon.

I should more carefully read release notes :D Thanks for pointing out to this ;)

@sapk

This comment has been minimized.

Copy link
Member

commented Sep 6, 2019

@lafriks go module is a good point providing a standard and more secure deps management but they are few quirks to get around and I also don't get some.

techknowlogick and others added 3 commits Sep 8, 2019
@techknowlogick

This comment has been minimized.

Copy link
Member Author

commented Sep 9, 2019

Currently blocked on: golangci/golangci-lint#663

@guillep2k

This comment has been minimized.

Copy link
Member

commented Sep 9, 2019

Currently blocked on: golangci/golangci-lint#663

Just in case, that issue was marked as duplicate of golangci/golangci-lint#659

...which was suggested as duplicate of golangci/golangci-lint#652

@codecov-io

This comment has been minimized.

Copy link

commented Sep 11, 2019

Codecov Report

Merging #8088 into master will increase coverage by <.01%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #8088      +/-   ##
==========================================
+ Coverage   41.84%   41.84%   +<.01%     
==========================================
  Files         482      482              
  Lines       64592    64592              
==========================================
+ Hits        27028    27030       +2     
+ Misses      34094    34093       -1     
+ Partials     3470     3469       -1
Impacted Files Coverage Δ
models/unit.go 62.16% <0%> (-5.41%) ⬇️
models/repo_list.go 74.14% <0%> (+0.97%) ⬆️
modules/log/event.go 65.64% <0%> (+1.02%) ⬆️

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 d0ad47b...83b1516. Read the comment docs.

@techknowlogick

This comment has been minimized.

Copy link
Member Author

commented Sep 11, 2019

deps have been upgraded and build now passes. In addition to golangci-lint, I also update swagger.

Please review :)

@sapk
sapk approved these changes Sep 11, 2019

@GiteaBot GiteaBot added lgtm/need 1 and removed lgtm/need 2 labels Sep 11, 2019

@GiteaBot GiteaBot added lgtm/done and removed lgtm/need 1 labels Sep 12, 2019

@lafriks lafriks merged commit 3f5cdfe into go-gitea:master Sep 12, 2019

2 checks passed

approvals/lgtm this commit looks good
continuous-integration/drone/pr Build is passing
Details
@guillep2k

This comment has been minimized.

Copy link
Member

commented Sep 12, 2019

Can we backport this? As I understand it, #8124 is depending on it.

@lunny

This comment has been minimized.

Copy link
Member

commented Sep 12, 2019

I don't think this should be backport to v1.9. It's a big change.

@guillep2k

This comment has been minimized.

Copy link
Member

commented Sep 12, 2019

@lunny You are right; the one I need was #8087

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
7 participants
You can’t perform that action at this time.