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

chore: Bump to GoLang v1.21 #2195

Merged
merged 3 commits into from
Jan 19, 2024
Merged

chore: Bump to GoLang v1.21 #2195

merged 3 commits into from
Jan 19, 2024

Conversation

shahzadlone
Copy link
Member

@shahzadlone shahzadlone commented Jan 12, 2024

Relevant issue(s)

Description

  • This is a routine version bump of GoLang, the previous bump was done in (chore: Bump to GoLang v1.20 #1689)
  • Also updates the golang version for AWS AMI generation.
  • Updated the ONLY_DEFRADB_REPO_CI_PAT token as the terraform build would fail otherwise as the token expired starting of this year.

@shahzadlone shahzadlone added the bump Bumped version for something label Jan 12, 2024
@shahzadlone shahzadlone added this to the DefraDB v0.9 milestone Jan 12, 2024
@shahzadlone shahzadlone requested a review from a team January 12, 2024 03:21
@shahzadlone shahzadlone self-assigned this Jan 12, 2024
Copy link

codecov bot commented Jan 12, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (58dc727) 74.19% compared to head (2f68943) 74.32%.
Report is 3 commits behind head on develop.

❗ Current head 2f68943 differs from pull request most recent head 615b969. Consider uploading reports for the commit 615b969 to get more accurate results

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff             @@
##           develop    #2195      +/-   ##
===========================================
+ Coverage    74.19%   74.32%   +0.13%     
===========================================
  Files          256      256              
  Lines        25508    25508              
===========================================
+ Hits         18924    18957      +33     
+ Misses        5290     5265      -25     
+ Partials      1294     1286       -8     
Flag Coverage Δ
all-tests 74.32% <ø> (+0.13%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

see 12 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d07bfd1...615b969. Read the comment docs.

Copy link
Contributor

@AndrewSisley AndrewSisley left a comment

Choose a reason for hiding this comment

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

LGTM :)

Copy link
Contributor

@AndrewSisley AndrewSisley left a comment

Choose a reason for hiding this comment

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

Actually wait

@AndrewSisley
Copy link
Contributor

AndrewSisley commented Jan 12, 2024

I wanted to quickly undo my approval sorry - I think this should be merged after the release. We gain nothing by including this in 0.9, and it is a sudden (and uncommunicated) break from our Go version policy and could be really annoying for some users.

Much better to wait until just after 0.9 is cut IMO, is only a day or so to wait.

@shahzadlone
Copy link
Member Author

shahzadlone commented Jan 12, 2024

I wanted to quickly undo my approval sorry - I think this should be merged after the release. We gain nothing by including this in 0.9, and it is a sudden (and uncommunicated) break from our Go version policy and could be really annoying for some users.

Much better to wait until just after 0.9 is cut IMO, is only a day or so to wait.

Thanks for your input, I have a strong disagreement for the following reasons:

  1. The release binaries are unaffected regardless of which version is used.

  2. Most likely, those who want to build from source, would be using a branch anyway.

  3. This break is quite minimal in comparison to other breaking changes we have pushed in this release (which will also be communicated through the similar channels as this one, i.e. commit log message and by updating of documents everywhere).

  4. Most importantly, in 2 weeks go is scheduled to release there v1.22 version. Our next release is going to be roughly in 8 weeks. Now that means users who don't follow (2) and want to build source at that exact release commit, will now be using an "unsupported version of GO" for at least the remaining 6 weeks until we make a new release: Each major Go release is supported until there are two newer major releases. For example, Go 1.5 was supported until the Go 1.7 release, and Go 1.6 was supported until the Go 1.8 release. https://go.dev/doc/devel/release

So in my opinion we do gain something by including this in 0.9 🤣

@fredcarle
Copy link
Collaborator

At this stage I'm quite ok with updating to 1.21. To most of our partners, at least to my knowledge, the impact will be negligible as they mostly use pre-compile binaries.

Copy link
Collaborator

@fredcarle fredcarle left a comment

Choose a reason for hiding this comment

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

I'm good with this change :)

@AndrewSisley
Copy link
Contributor

Thanks for your input...

RE:
(1) - I was talking about embedded users.
(2) - I don't think we can claim this. And even if we were able to accurately track that that doesn't mean we should not care about anyone actually using the releases.
(3) - Other breaking changes are documented and expected. They also don't require people to update their own runtime, possibly to a version that could be incompatible with other stuff they are using leaving them stuck on 0.8.
(4) - This is an inherent weakness in our Go version policy and not specific to this PR, are you suggesting we abandon it permanently?

I won't block this, but I will not approve it either (dismiss my review if you want to merge). I don't understand why you are rushing to merge this a few hours early (my understanding is that the only reason for breaking process with this is for ACP, and ACP is not going to be snuck into the 0.9 release anyway).

@shahzadlone
Copy link
Member Author

Discussed on discord, will wait for the release to merge.

@source-devs

This comment was marked as resolved.

Copy link
Contributor

@AndrewSisley AndrewSisley left a comment

Choose a reason for hiding this comment

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

0.9 Released, approving :)

@source-devs
Copy link

Terraform Format and Style success

Terraform Initialization success

Terraform Validation success

Terraform Plan success

Show Plan

Terraform Plan Output:
data.aws_ami.ami: Reading...
aws_security_group.sg: Refreshing state... [id=sg-03ba6f1f9cd118f43]
data.aws_ami.ami: Read complete after 0s [id=ami-0b436d9dc6b1762be]
aws_instance.instance: Refreshing state... [id=i-087b6a34e288d739d]

No changes. Your infrastructure matches the configuration.

Terraform has compared your real infrastructure against your configuration
and found no differences, so no changes are needed.


Pushed By: @shahzadlone
SHA: 615b9693eea59a9f8e22363cc018cd44198a4d7a

@sourcenetwork sourcenetwork deleted a comment from source-devs Jan 19, 2024
@shahzadlone shahzadlone merged commit 6d4bcc7 into develop Jan 19, 2024
28 of 29 checks passed
@shahzadlone shahzadlone deleted the lone/bump-golang-to-1-21 branch January 19, 2024 15:27
shahzadlone added a commit to shahzadlone/defradb that referenced this pull request Jan 22, 2024
## Relevant issue(s)
- Resolves sourcenetwork#2194
- Resolves sourcenetwork#2196


## Description
- This is a routine version bump of GoLang, the previous bump was done
in (sourcenetwork#1689)
- Also updates the golang version for AWS AMI generation.
- Updated the `ONLY_DEFRADB_REPO_CI_PAT` token as the terraform build
would fail otherwise as the token expired starting of this year.
nasdf pushed a commit to nasdf/defradb that referenced this pull request Jan 23, 2024
## Relevant issue(s)
- Resolves sourcenetwork#2194
- Resolves sourcenetwork#2196


## Description
- This is a routine version bump of GoLang, the previous bump was done
in (sourcenetwork#1689)
- Also updates the golang version for AWS AMI generation.
- Updated the `ONLY_DEFRADB_REPO_CI_PAT` token as the terraform build
would fail otherwise as the token expired starting of this year.
shahzadlone added a commit to shahzadlone/defradb that referenced this pull request Feb 23, 2024
## Relevant issue(s)
- Resolves sourcenetwork#2194
- Resolves sourcenetwork#2196


## Description
- This is a routine version bump of GoLang, the previous bump was done
in (sourcenetwork#1689)
- Also updates the golang version for AWS AMI generation.
- Updated the `ONLY_DEFRADB_REPO_CI_PAT` token as the terraform build
would fail otherwise as the token expired starting of this year.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bump Bumped version for something
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update token ONLY_DEFRADB_REPO_CI_PAT expired end of 2023 on dev account chore: Bump to GoLang v1.21
4 participants