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

CI builds validate against last two versions of Go, dropping 1.14 and adding 1.16 #1865

Merged
merged 2 commits into from
May 6, 2021

Conversation

gliptak
Copy link
Contributor

@gliptak gliptak commented Apr 29, 2021

No description provided.

@MrAlias MrAlias added the Skip Changelog PRs that do not require a CHANGELOG.md entry label Apr 30, 2021
.github/workflows/ci.yml Outdated Show resolved Hide resolved
@Aneurysm9
Copy link
Member

Can you update the list of tested versions in the README?

@gliptak
Copy link
Contributor Author

gliptak commented Apr 30, 2021

@Aneurysm9 updated README

@codecov
Copy link

codecov bot commented Apr 30, 2021

Codecov Report

Merging #1865 (acc7cdf) into main (cbcd4b1) will increase coverage by 0.2%.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##            main   #1865     +/-   ##
=======================================
+ Coverage   79.0%   79.2%   +0.2%     
=======================================
  Files        139     139             
  Lines       7477    7459     -18     
=======================================
+ Hits        5909    5910      +1     
+ Misses      1322    1303     -19     
  Partials     246     246             
Impacted Files Coverage Δ
exporters/trace/jaeger/uploader.go 50.6% <0.0%> (-0.1%) ⬇️
sdk/export/metric/exportkind_string.go 0.0% <0.0%> (ø)
metric/number/kind_string.go 33.3% <0.0%> (+16.6%) ⬆️
attribute/type_string.go 33.3% <0.0%> (+23.3%) ⬆️
metric/instrumentkind_string.go 33.3% <0.0%> (+23.3%) ⬆️

@MrAlias MrAlias removed the Skip Changelog PRs that do not require a CHANGELOG.md entry label May 2, 2021
@MrAlias
Copy link
Contributor

MrAlias commented May 2, 2021

I had initially added that this can skip the change log, but if we are going to change the minimum version of Go that we support we definitely need to include that in the changelog.

@MrAlias
Copy link
Contributor

MrAlias commented May 2, 2021

I'm a bit hesitant to make this change. By moving the minimum version number from 1.14 to 1.15 we are reducing the versions of Go we support and consequentially the number of users we can support here. If our goal is to promote as wide adoption of the project as we can achieve, this goes against that goal.

… adding 1.16

Signed-off-by: Gábor Lipták <gliptak@gmail.com>
@gliptak gliptak changed the title Add Go 1.16 into CI matrix CI builds validate against last two versions of Go, dropping 1.14 and adding 1.16 May 2, 2021
@gliptak
Copy link
Contributor Author

gliptak commented May 2, 2021

#1064

@evantorrie
Copy link
Contributor

I'm a bit hesitant to make this change. By moving the minimum version number from 1.14 to 1.15 we are reducing the versions of Go we support and consequentially the number of users we can support here. If our goal is to promote as wide adoption of the project as we can achieve, this goes against that goal.

Given that there may be issues related to changes in go-1.16 module handling, perhaps running tests across the current and two past versions of go is a reasonable solution.

@lizthegrey
Copy link
Member

lizthegrey commented May 3, 2021

I'm a bit hesitant to make this change. By moving the minimum version number from 1.14 to 1.15 we are reducing the versions of Go we support and consequentially the number of users we can support here. If our goal is to promote as wide adoption of the project as we can achieve, this goes against that goal.

Supporting 2 versions of golang is in keeping with ecosystem standards, and it's worth noting that we aren't yet GA. once we're GA I could see the argument for supporting .15, .16, and .17 in parallel to accommodate slower movers. Remember golang comes out once per 6 months so that's 12 months of official support per major version (plus 6 months if we support N-2 and not just N-1 once hitting GA).

https://www.reddit.com/r/golang/comments/cmb5bp/is_there_an_lts_version_of_go/

@lizthegrey
Copy link
Member

@Aneurysm9 this is going to need your approval to run the CI and then merge

@MrAlias
Copy link
Contributor

MrAlias commented May 6, 2021

From SIG meeting:

  • When we release 1.0 we will want to pin our minimum supported version of Go.
  • We are not there now, so let's upgrade and keep a floating support window of the current and last minor versions.

@Aneurysm9 Aneurysm9 merged commit d20e722 into open-telemetry:main May 6, 2021
@gliptak gliptak deleted the patch-1 branch May 6, 2021 21:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants