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

Update to Go 1.17 #26

Merged
merged 3 commits into from
Feb 5, 2022
Merged

Conversation

andrerfcsantos
Copy link
Member

@andrerfcsantos andrerfcsantos commented Nov 9, 2021

@andrerfcsantos andrerfcsantos requested a review from a team as a code owner November 9, 2021 00:12
@andrerfcsantos andrerfcsantos added the x:size/tiny Tiny amount of work label Nov 9, 2021
@andrerfcsantos andrerfcsantos marked this pull request as draft November 9, 2021 00:52
@andrerfcsantos andrerfcsantos marked this pull request as ready for review November 9, 2021 22:06
Copy link
Member

@junedev junedev left a comment

Choose a reason for hiding this comment

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

Sorry that it took me so long to start reviewing this. Here my initial findings:

  • This repo includes dependencies. Please run go mod vendor and go mod tidy (with go 1.17) to adjust the go.mod and go.sum file to the 1.17 version. (Without that, it gives me an error go: vendored module github.com/davecgh/go-spew@v1.1.1 should be required explicitly in go.mod)
  • Would be great if you could also add the vendor folder to the gitignore file, it is currently missing there.
  • Did you get the tests to pass? If yes, how? They fail for me. However this seems unrelated to the Go version update.
    (I might add more comments later if anything else comes up.)

@andrerfcsantos
Copy link
Member Author

Thanks for taking a look at this.

I'll go over this in more detail over the weekend and will keep you posted.

@andrerfcsantos
Copy link
Member Author

@junedev Ran go mod vendor && go mod tidy and also updated gitignore to include the vendoring directory and binaries from go build.

I ran the tests with go test and they do seem to be failing. However, they are failing for the same reason in our current main. Seems like something we must look at, but I don't think it should be a blocker for this PR.

@junedev
Copy link
Member

junedev commented Feb 5, 2022

Thanks for the update and checking regarding the failing test. Feel free to merge this but please create an issue for the failing test cases if there isn't already one.

@andrerfcsantos andrerfcsantos merged commit 3723779 into exercism:main Feb 5, 2022
@andrerfcsantos
Copy link
Member Author

Issue for the failing tests: #29

@junedev junedev added x:size/small Small amount of work and removed x:size/tiny Tiny amount of work labels Feb 6, 2022
@andrerfcsantos andrerfcsantos mentioned this pull request Mar 15, 2022
@andrerfcsantos andrerfcsantos deleted the update-go branch April 3, 2022 23:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
x:size/small Small amount of work
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants