Update to Go 1.18 #72

pedreviljoen · 2022-04-12T19:38:42Z

As per below issue:

This PR updates Go to version 1.18

coveralls · 2022-04-12T19:46:09Z

Pull Request Test Coverage Report for Build 2313365415

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

For more information on this, see Tracking coverage changes with pull request builds.
To avoid this issue with future PRs, see these Recommended CI Configurations.
For a quick fix, rebase this PR at GitHub. Your next report should be accurate.

Details

0 of 0 changed or added relevant lines in 0 files are covered.
No unchanged relevant lines lost coverage.
Overall coverage remained the same at 77.309%

Totals
Change from base Build 2309967076:	0.0%
Covered Lines:	385
Relevant Lines:	498

💛 - Coveralls

andrerfcsantos · 2022-04-12T20:31:58Z

@pedreviljoen Thanks for this.

This PR can't be merged just yet. The reason is that we are using golangci/golangci-lint-action@v2 to run golangci-lint at v1.43 with the default checks:

go-test-runner/.github/workflows/test.yml

Lines 16 to 19 in 5670f9f

    
           - name: Run linters 
        
             uses: golangci/golangci-lint-action@v2 
        
             with: 
        
               version: v1.43

However, golangci-lint in v1.43 doesn't understand generics. The support was only added in v1.45. Bumping the version of golangci-lint will probably make CI pass.

However, there's another nuance to this. At version 1.45.2, golangci-lint uses the version v0.2.2 of honnef.co/go/tools which provides the staticcheck check:

https://github.com/golangci/golangci-lint/blob/8bdc4d3f8044b1a20e10a9f519b5f738e8188877/go.mod#L99

This check is used in our Go repositories, and it is enabled by default by golangci-lint. But support for generics was only added in v0.3.0 of staticcheck. Checking the repository of golangci-lint, I see they already updated to v0.3.0 of staticcheck on master:

https://github.com/golangci/golangci-lint/blob/ff93f2715709ab084d0004b197f5ba84116a65ee/go.mod#L101

However, this is only on master, it's not on any of the official releases. It is expected to be included on the next release of golangci-lint. I would wait for the next release golangci-lint to contain v0.3.0 of staticcheck and only update then.

This should also be merged together with the update to the analyzer.

.gitignore

pedreviljoen · 2022-04-13T07:02:12Z

@andrerfcsantos
Ok cool, let's wait for those peer dependencies to be more stable. Is there a way we can keep an eye on those?

andrerfcsantos · 2022-04-13T08:56:56Z

The best way is to keep track of new releases of golangci/golangci-lint

There's also this issue keeping track of the support for 1.18 of all the linters of golangci-lint: golangci/golangci-lint#2649

In our case, I think it's fine to wait just for the next release of golangci-lint to update staticcheck to support go 1.18, the other are not as important. Also, like that issue describes, this problem with lack of support only happens when the go.mod for the module being checked declares 1.18 - we don't have any currently in the main repo for the Go track and I won't think we'll have one for some time. Even when we do, I think having just staticcheck (and some other basic checks) is enough and we shouldn't block students to use Go 1.18 just because we are waiting for perfect support for go 1.18 in the tools for our CI (which doesn't affect students directly). We can always do a second upgrade later when those tools add more support for go 1.18.

andrerfcsantos · 2022-05-09T18:27:35Z

@pedreviljoen golangci-lint released an update today (1.46.0) that updates staticcheck to work with Go 1.18: https://github.com/golangci/golangci-lint/releases/tag/v1.46.0

The relevant changelog lines for us are:

golangci/golangci-lint@e4945f7 build(deps): bump honnef.co/go/tools from 0.2.2 to 0.3.0 (golangci/golangci-lint#2738)
golangci/golangci-lint@5c77c1b build(deps): bump honnef.co/go/tools from 0.3.0 to 0.3.1 (golangci/golangci-lint#2780)
[...]
golangci/golangci-lint@f5b92e1 staticcheck: re-enable for go1.18 (golangci/golangci-lint#2746)

So it would be great if you could update this PR to use golangci-lint v1.46.0. It's probably also a good idea to update the versions for our actions:

actions/setup-go@v2 -> actions/setup-go@v3
golangci/golangci-lint-action@v2 -> golangci/golangci-lint-action@v3

@pedreviljoen Let us know if you are still interested in finishing this.

pedreviljoen · 2022-05-09T19:56:06Z

@andrerfcsantos
Ok cool will do so tomorrow, while doing the upgrade to 1.18 I see there are two other open requests too. Will perhaps do all 3 then 😅

pedreviljoen · 2022-05-12T12:23:14Z

@andrerfcsantos
Implemented the recommended updates, let me know if you need anything else?

andrerfcsantos

@pedreviljoen The tests for the repo are now passing, which is great! This is definitively an improvement.

My only suggestion would only be to convert all actions/setup-go@v2 to actions/setup-go@v3 not just the one for the "tests" job - sorry if I didn't make it clear earlier. In addition, convert all instances of actions/checkout@v2 to actions/checkout@v3 - just noticed they also have a v3 now.

andrerfcsantos · 2022-05-12T20:46:32Z

@pedreviljoen We'll gladly let you take care of the upgrades for the other repos too, thanks for the help!

Those upgrades should be similar to this one, where we should also upgrade actions/setup-go, actions/checkout and golangci/golangci-lint-action there too where applicable.

The only difference to this PR that comes to mind is that in the main repo (exercism/go), we are installing golangci-lint via a script and not an action, so there the version of golangci-lint must be changed by changing the URL in the script and not by changing an action configuration. And bumping the version of go.mod files that are part of tests is ok, but we should not do it for the go.mod files of student's exercises, specially if the exercise doesn't require generics or anything else in 1.18.

andrerfcsantos · 2022-05-25T08:39:48Z

@pedreviljoen Still interested in working on this? Totally ok if you are not, just let us know and we can finish it for you. Otherwise, we are happy to let you finish.

pedreviljoen · 2022-05-26T07:16:20Z

@andrerfcsantos
Thanks for pinging me, got busy with a project at work. Will look into the mentioned suggestions and feed back 👍

andrerfcsantos

Seems like the pipeline is failing now on gcov2lcov. My suggestion would be to upgrade those versions too to see if the problem is fixed:

jandelgado/gcov2lcov-action@v1.0.6 -> jandelgado/gcov2lcov-action@v1.0.9
coverallsapp/github-action@v1.1.2 -> coverallsapp/github-action@v1.1.3

pedreviljoen · 2022-05-30T18:48:33Z

@andrerfcsantos
All good, updated those coverage jobs failing

.github/workflows/test.yml

andrerfcsantos · 2022-05-31T07:30:44Z

That seems to have fixed it, cool!

Just waiting for more approvals and for the other tooling PRs before we can merge this one.

Having also the upgrade of the analyzer to 1.18 at least would be important for us to say that the track supports 1.18. Would you also like to work on that one?

pedreviljoen · 2022-06-01T06:17:29Z

@andrerfcsantos
yeah cool, will do 👍

pedreviljoen · 2022-06-17T08:07:04Z

@andrerfcsantos
I think we are in a good place now?

analyzer and representer also bumped to 1.18

andrerfcsantos · 2022-06-17T08:08:09Z

Yeah. We now just need a review from @exercism/maintainers-admin

ErikSchierboom · 2022-06-17T08:33:40Z

Is Go 1.18 backwards compatible with the currently used version (1.17 I think)?

andrerfcsantos · 2022-06-17T08:50:08Z

@ErikSchierboom Yes. New 1.x releases have the implicit promise of being compatible with all previous 1.x versions of Go. This is known as the "Go 1 compatibility promise", and it's detailed here Go 1 and the Future of Go Programs
.

It is intended that programs written to the Go 1 specification will continue to compile and run correctly, unchanged, over the lifetime of that specification. At some indefinite point, a Go 2 specification may arise, but until that time, Go programs that work today should continue to work even as future "point" releases of Go 1 arise (Go 1.1, Go 1.2, etc.).

The other related PRs:

ErikSchierboom · 2022-06-17T10:07:43Z

.github/workflows/test.yml

@@ -8,31 +8,31 @@ jobs:
    runs-on: ubuntu-latest
    steps:
    - name: Install Go
-      uses: actions/setup-go@v2
+      uses: actions/setup-go@v3


Our GHA recommendations guide has a section on pinning actions to SHAs, for security purposes. Admittedly, the actions/ actions are less likely to be affected, but this workflow also uses some workflows developed by third-parties, for which I'd definitely recommend pinning them. It's totally fine to do this in a follow-up PR.

andrerfcsantos · 2022-07-09T22:17:58Z

@ErikSchierboom I'll do follow-up PRs then :)

chore: bumped go to version 1.18

3df791c

pedreviljoen requested a review from a team as a code owner April 12, 2022 19:38

junedev reviewed Apr 12, 2022

View reviewed changes

.gitignore Outdated Show resolved Hide resolved

chore: PR comments for global gitignore

ba7e624

pedreviljoen and others added 2 commits May 12, 2022 14:15

Merge branch 'exercism:main' into main

e59da9e

added: updated golang lint action version

1c74e96

pedreviljoen requested a review from junedev May 12, 2022 12:23

andrerfcsantos requested changes May 12, 2022

View reviewed changes

added: bump action version for setup-go & checkout

9d771dd

pedreviljoen requested a review from andrerfcsantos May 26, 2022 07:18

andrerfcsantos requested changes May 26, 2022

View reviewed changes

fix: code coverage failing unit tests

bf21505

andrerfcsantos requested changes May 30, 2022

View reviewed changes

.github/workflows/test.yml Outdated Show resolved Hide resolved

fix: code coverage failing unit tests

c93c267

pedreviljoen requested a review from andrerfcsantos May 31, 2022 06:42

andrerfcsantos approved these changes May 31, 2022

View reviewed changes

ErikSchierboom approved these changes Jun 17, 2022

View reviewed changes

andrerfcsantos merged commit 1b711ce into exercism:main Jul 9, 2022

andrerfcsantos mentioned this pull request Jul 10, 2022

Pin actions to SHAs #76

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update to Go 1.18 #72

Update to Go 1.18 #72

pedreviljoen commented Apr 12, 2022

coveralls commented Apr 12, 2022 •

edited

Loading

andrerfcsantos commented Apr 12, 2022 •

edited

Loading

pedreviljoen commented Apr 13, 2022

andrerfcsantos commented Apr 13, 2022

andrerfcsantos commented May 9, 2022

pedreviljoen commented May 9, 2022

pedreviljoen commented May 12, 2022

andrerfcsantos left a comment •

edited

Loading

andrerfcsantos commented May 12, 2022 •

edited

Loading

andrerfcsantos commented May 25, 2022

pedreviljoen commented May 26, 2022

andrerfcsantos left a comment

pedreviljoen commented May 30, 2022

andrerfcsantos commented May 31, 2022

pedreviljoen commented Jun 1, 2022

pedreviljoen commented Jun 17, 2022

andrerfcsantos commented Jun 17, 2022

ErikSchierboom commented Jun 17, 2022

andrerfcsantos commented Jun 17, 2022

ErikSchierboom Jun 17, 2022

andrerfcsantos commented Jul 9, 2022

Update to Go 1.18 #72

Update to Go 1.18 #72

Conversation

pedreviljoen commented Apr 12, 2022

coveralls commented Apr 12, 2022 • edited Loading

Pull Request Test Coverage Report for Build 2313365415

Warning: This coverage report may be inaccurate.

Details

💛 - Coveralls

andrerfcsantos commented Apr 12, 2022 • edited Loading

pedreviljoen commented Apr 13, 2022

andrerfcsantos commented Apr 13, 2022

andrerfcsantos commented May 9, 2022

pedreviljoen commented May 9, 2022

pedreviljoen commented May 12, 2022

andrerfcsantos left a comment • edited Loading

Choose a reason for hiding this comment

andrerfcsantos commented May 12, 2022 • edited Loading

andrerfcsantos commented May 25, 2022

pedreviljoen commented May 26, 2022

andrerfcsantos left a comment

Choose a reason for hiding this comment

pedreviljoen commented May 30, 2022

andrerfcsantos commented May 31, 2022

pedreviljoen commented Jun 1, 2022

pedreviljoen commented Jun 17, 2022

andrerfcsantos commented Jun 17, 2022

ErikSchierboom commented Jun 17, 2022

andrerfcsantos commented Jun 17, 2022

ErikSchierboom Jun 17, 2022

Choose a reason for hiding this comment

andrerfcsantos commented Jul 9, 2022

coveralls commented Apr 12, 2022 •

edited

Loading

andrerfcsantos commented Apr 12, 2022 •

edited

Loading

andrerfcsantos left a comment •

edited

Loading

andrerfcsantos commented May 12, 2022 •

edited

Loading