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

[Merged by Bors] - Require golang >= 1.15 #2452

Closed
wants to merge 1 commit into from
Closed

[Merged by Bors] - Require golang >= 1.15 #2452

wants to merge 1 commit into from

Conversation

lrettig
Copy link
Member

@lrettig lrettig commented Jun 12, 2021

Motivation

Golang 1.15 has some nice features such as testing.TempDir.

Changes

This PR updates CI and scripts to require go >= 1.15.

Test Plan

N/A

TODO

  • Explain motivation or link existing issue(s)
  • Test changes and document test plan
  • Update documentation as needed

DevOps Notes

  • This PR does not require configuration changes (e.g., environment variables, GitHub secrets, VM resources)
  • This PR does not affect public APIs
  • This PR does not rely on a new version of external services (PoET, elasticsearch, etc.)
  • This PR does not make changes to log messages (which monitoring infrastructure may rely on)

@dshulyak
Copy link
Contributor

@lrettig any reason not to go for 1.16?

@noamnelke
Copy link
Member

any reason not to go for 1.16?

In general, we want to support older versions, as long as we don't use features from the newer version. This is not to say that you can't use newer versions, or that we shouldn't support those, too. That's why #2475 is a good idea.

@dshulyak
Copy link
Contributor

In general, we want to support older versions, as long as we don't use features from the newer version. This is not to say that you can't use newer versions, or that we shouldn't support those, too. That's why #2475 is a good idea.

I don't understand why, to be honest. It is not like it is hard to update golang locally. And there are always improvements/fixes under the hood, even if those are not new APIs. For instance, this is a very useful change for monitoring (among other changes in runtime):

On Linux, the runtime now defaults to releasing memory to the operating system promptly (using MADV_DONTNEED), rather than lazily when the operating system is under memory pressure (using MADV_FREE). This means process-level memory statistics like RSS will more accurately reflect the amount of physical memory being used by Go processes. Systems that are currently using GODEBUG=madvdontneed=1 to improve memory monitoring behavior no longer need to set this environment variable. 

@noamnelke
Copy link
Member

@dshulyak the improvement you mention really seems useful and we should therefore upgrade to Go 1.16 in our testnet infra, but it doesn't depend on us requiring it... If the code works on 1.15, that should be the minimum.

While we don't see that many contributions from the community, we're still an open source project and we don't want to force developers to upgrade their Go for no reason, other than "a newer version exists".

@lrettig
Copy link
Member Author

lrettig commented Jun 23, 2021 via email

bors bot pushed a commit that referenced this pull request Jun 23, 2021
## Motivation
Golang 1.15 has some [nice features](https://golang.org/doc/go1.15) such as `testing.TempDir`.

## Changes
This PR updates CI and scripts to require go >= 1.15.

## Test Plan
N/A

## TODO
<!-- This section should be removed when all items are complete -->
- [x] Explain motivation or link existing issue(s)
- [ ] Test changes and document test plan
- [x] Update documentation as needed

## DevOps Notes
<!-- Please uncheck these items as applicable to make DevOps aware of changes that may affect releases -->
- [x] This PR does not require configuration changes (e.g., environment variables, GitHub secrets, VM resources)
- [x] This PR does not affect public APIs
- [x] This PR does not rely on a new version of external services (PoET, elasticsearch, etc.)
- [x] This PR does not make changes to log messages (which monitoring infrastructure may rely on)
@bors
Copy link

bors bot commented Jun 23, 2021

Pull request successfully merged into develop.

Build succeeded:

@bors bors bot changed the title Require golang >= 1.15 [Merged by Bors] - Require golang >= 1.15 Jun 23, 2021
@bors bors bot closed this Jun 23, 2021
@bors bors bot deleted the go-115 branch June 23, 2021 13:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants