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.

Sign up for GitHub

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

Jump to bottom

chore: upgrade from Go 1.16 to 1.17 #479

Merged
merged 5 commits into from
Mar 7, 2022
Merged

chore: upgrade from Go 1.16 to 1.17 #479

merged 5 commits into from
Mar 7, 2022

Conversation

rootulp
Copy link
Contributor

@rootulp rootulp commented Feb 24, 2022

Fixes #472

@rootulp rootulp mentioned this pull request Feb 24, 2022
Closed
liamsi
liamsi reviewed Feb 24, 2022
View reviewed changes
Copy link
Member

@liamsi liamsi left a comment

Choose a reason for hiding this comment

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

Thanks for this! I do think it makes sense to also update the other places you've mentioned in #472 (comment) but I would leave it up to the core team to review this and decide on this.

@renaynay
Copy link
Member

We should update go.mod to 1.17 as well as libs/fslock/lock_unix.go to remove the // +build directive as the new build directive for 1.17 is go:build

@rootulp
chore: upgrade to Go 1.17
b8e3090 
Testing
```
make install # succeeds
make build # succeeds
```
rootulp
rootulp commented Feb 25, 2022
View reviewed changes
README.md Show resolved Hide resolved
go.mod Show resolved Hide resolved
@rootulp rootulp changed the title chore: remove Go 1.16 from GH actions chore: upgrade from Go 1.16 to 1.17 Feb 25, 2022
Wondertan
Wondertan approved these changes Mar 4, 2022
View reviewed changes
Copy link
Member

@Wondertan Wondertan left a comment

Choose a reason for hiding this comment

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

Awesome! The v1.17's lazyloading feature is something I was waiting for a long time in Go and excited to finally have it in this repo. Thank you @rootulp. LGTM.

@Wondertan
Copy link
Member

Wondertan commented Mar 4, 2022 •
edited
Loading

@liamsi, as you added the no_changelog tag, why do you think the change does not deserve a changelog entry? Even if it is a small change it has a bigger impact.

@adlerjohn
Copy link
Member

+1 this is a breaking change and should have a changelog entry

@rootulp
Copy link
Contributor Author

rootulp commented Mar 7, 2022

@liamsi added no_changelog when this PR just removed 1.16 from GH actions. Sorry this PR has increased in scope since then so I added a changelog. Please let me know if there's anything else I can do.

@Wondertan
Copy link
Member

@rootulp, can you pls remove your merge commit? We always rebase and avoid those

@renaynay renaynay requested a review from liamsi March 7, 2022 11:20
renaynay
renaynay approved these changes Mar 7, 2022
View reviewed changes
Copy link
Member

@renaynay renaynay left a comment

Choose a reason for hiding this comment

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

Approved, thanks for your contribution @rootulp

We will squash merge the PR :)

@renaynay renaynay merged commit e943732 into celestiaorg:main Mar 7, 2022
@rootulp rootulp deleted the rp/remove-go-1.16 branch March 7, 2022 16:27
toanngosy pushed a commit to toanngosy/celestia-node that referenced this pull request Mar 7, 2022
@rootulp @toanngosy
chore: upgrade from Go 1.16 to 1.17 (celestiaorg#479)
53ea1c7 
* chore: remove Go 1.16 from GH actions

Fixes celestiaorg#472

* chore: upgrade to Go 1.17

Testing
```
make install # succeeds
make build # succeeds
```

* Add back Go 1.17 preferred build directive

Motivation in
https://github.com/celestiaorg/celestia-node/pull/479/files#r816032325
toanngosy pushed a commit to toanngosy/celestia-node that referenced this pull request Mar 8, 2022
@rootulp @toanngosy
chore: upgrade from Go 1.16 to 1.17 (celestiaorg#479)
444924a 
* chore: remove Go 1.16 from GH actions

Fixes celestiaorg#472

* chore: upgrade to Go 1.17

Testing
```
make install # succeeds
make build # succeeds
```

* Add back Go 1.17 preferred build directive

Motivation in
https://github.com/celestiaorg/celestia-node/pull/479/files#r816032325
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@renaynay renaynay renaynay approved these changes

@Wondertan Wondertan Wondertan approved these changes

@liamsi liamsi Awaiting requested review from liamsi

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Change Request]: remove 1.16 from github actions
5 participants
@rootulp @renaynay @Wondertan @adlerjohn @liamsi