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: Bump to GoLang v1.19 #818

Merged
merged 7 commits into from
Feb 15, 2023
Merged

chore: Bump to GoLang v1.19 #818

merged 7 commits into from
Feb 15, 2023

Conversation

shahzadlone
Copy link
Member

@shahzadlone shahzadlone commented Sep 18, 2022 •
edited
Loading

Relevant issue(s)

Resolves #817
Resolves #1070

Description

This PR was initially started as: ci: Bump workflow's golang-ci lint version to fix the golang-ci lint issue. This work that was done in this PR was the following:

  • Remove the deadcode and varcheck linters that are now deprecated with v1.49
  • Remove all nolint directives where the deprecated linters were being referenced.
  • Update CI's linter version to use the latest version.

However as the golang-ci lint team still didn't fix that issue but golang1.19 was released we can just fix those errors and bump the GoLang version for defradb and keeping the previous linter fixes.

Inaddtion also re-enabling the goheader linter rule as the change was somehow dropped when we released defradb v0.4.

How has this been tested?

Local & CI

Specify the platform(s) on which this was tested:

  • Manjaro Wsl 2

@shahzadlone shahzadlone added action/no-benchmark Skips the action that runs the benchmark. deprecate Indicates something is deprecated. bump Bumped version for something labels Sep 18, 2022
@shahzadlone shahzadlone added this to the DefraDB v0.4 milestone Sep 18, 2022
@shahzadlone shahzadlone requested a review from a team September 18, 2022 15:23
@shahzadlone shahzadlone self-assigned this Sep 18, 2022
@shahzadlone shahzadlone changed the title chore: Bump linter to version v1.49 and remove deprecated linters chore: Bump linter version to v1.49 Sep 18, 2022
@shahzadlone shahzadlone marked this pull request as draft September 18, 2022 15:29
@shahzadlone
Copy link
Member Author

Copying my comment on #817 here:

The issue that made us hard code to version v1.47 still persists (#727) lint action uses the default built binary that enforces go v1.19 formatting.

suggest waiting for this until that is fixed (or else we would manually have to build golang-ci every time in the CI).

Or until we bump to GoLang v1.19

@shahzadlone shahzadlone changed the title chore: Bump linter version to v1.49 ci: Bump workflow's golang-ci lint version to v1.49 Sep 18, 2022
@orpheuslummis
Copy link
Contributor

Copying my comment on #817 here:

The issue that made us hard code to version v1.47 still persists (#727) lint action uses the default built binary that enforces go v1.19 formatting.

suggest waiting for this until that is fixed (or else we would manually have to build golang-ci every time in the CI).

Or until we bump to GoLang v1.19

That means CI check will fail with gofmt v1.19 errors ?

@shahzadlone
Copy link
Member Author

shahzadlone commented Sep 18, 2022 •
edited
Loading

Copying my comment on #817 here:
The issue that made us hard code to version v1.47 still persists (#727) lint action uses the default built binary that enforces go v1.19 formatting.
suggest waiting for this until that is fixed (or else we would manually have to build golang-ci every time in the CI).
Or until we bump to GoLang v1.19

That means CI check will fail with gofmt v1.19 errors ?

Yes, with the current golang-ci workflow action version we have and the actual hardcoded golangci version.

However once v0.3.1 is kicked out the door, will investigate if perhaps there are new golang-ci action versions and if they fix this bug. Until then suggest leaving this as draft.

@shahzadlone shahzadlone changed the title ci: Bump workflow's golang-ci lint version to v1.49 ci: Bump workflow's golang-ci lint version Oct 7, 2022
@shahzadlone shahzadlone force-pushed the lone/chore/bump-golang-ci-and-depricate-linters branch from 6d858bc to 30e5c44 Compare October 7, 2022 07:58
@shahzadlone
Copy link
Member Author

shahzadlone commented Oct 28, 2022 •
edited
Loading

Blocked until this is fixed: golangci/golangci-lint-action#540
Because of this issue: golangci/golangci-lint-action#535

@shahzadlone shahzadlone modified the milestones: DefraDB v0.4, DefraDB v0.5 Dec 13, 2022
@shahzadlone shahzadlone force-pushed the lone/chore/bump-golang-ci-and-depricate-linters branch from 30e5c44 to ccfcdad Compare December 17, 2022 23:47
@shahzadlone shahzadlone mentioned this pull request Feb 2, 2023
Closed
@shahzadlone shahzadlone force-pushed the lone/chore/bump-golang-ci-and-depricate-linters branch from ccfcdad to 7eb225b Compare February 2, 2023 02:18
@shahzadlone shahzadlone changed the title ci: Bump workflow's golang-ci lint version chore: Bump to GoLang v1.19 Feb 2, 2023
@shahzadlone shahzadlone marked this pull request as ready for review February 2, 2023 02:45
@shahzadlone shahzadlone force-pushed the lone/chore/bump-golang-ci-and-depricate-linters branch from bf2974d to 93e509e Compare February 2, 2023 03:07
@shahzadlone shahzadlone mentioned this pull request Feb 7, 2023
Closed
@shahzadlone shahzadlone force-pushed the lone/chore/bump-golang-ci-and-depricate-linters branch 2 times, most recently from 968fc54 to bcb2025 Compare February 7, 2023 16:12
@shahzadlone shahzadlone mentioned this pull request Feb 14, 2023
Merged
4 tasks
fredcarle
fredcarle approved these changes Feb 14, 2023
View reviewed changes
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.

LGTM

@orpheuslummis
Copy link
Contributor

Perhaps we can additionally drop support for Go 1.18 as it AFAIK not further supported by Official Go Team. This way we can additionally get rid of things like https://github.com/sourcenetwork/defradb/blob/develop/tests/integration/utils.go#L53 in this PR

@AndrewSisley
Copy link
Contributor

There is an init block in the main intergration test utils that needs removal within this PR too (unless we want to open a new issue for it), Orpheus noted it in #1070

@shahzadlone shahzadlone force-pushed the lone/chore/bump-golang-ci-and-depricate-linters branch from bcb2025 to 2deb84f Compare February 14, 2023 19:41
@shahzadlone
Copy link
Member Author

Perhaps we can additionally drop support for Go 1.18 as it AFAIK not further supported by Official Go Team. This way we can additionally get rid of things like https://github.com/sourcenetwork/defradb/blob/develop/tests/integration/utils.go#L53 in this PR

There is an init block in the main intergration test utils that needs removal within this PR too (unless we want to open a new issue for it), Orpheus noted it in #1070

Done

AndrewSisley
AndrewSisley reviewed Feb 14, 2023
View reviewed changes
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.

Unapproving as I missed the cid change, happy for you to merge if that has been resolved and someone else approves though

@shahzadlone shahzadlone force-pushed the lone/chore/bump-golang-ci-and-depricate-linters branch from 513da54 to f00aa58 Compare February 15, 2023 04:41
@shahzadlone
Copy link
Member Author

shahzadlone commented Feb 15, 2023 •
edited
Loading

Unapproving as I missed the cid change, happy for you to merge if that has been resolved and someone else approves though

This PR shouldn't merge until #1094 is in, in which case the cids won't change (I have dropped that commit btw).

@shahzadlone shahzadlone force-pushed the lone/chore/bump-golang-ci-and-depricate-linters branch from f00aa58 to b402550 Compare February 15, 2023 20:09
@shahzadlone shahzadlone merged commit 07dfd1e into develop Feb 15, 2023
@shahzadlone shahzadlone deleted the lone/chore/bump-golang-ci-and-depricate-linters branch February 15, 2023 20:29
shahzadlone added a commit that referenced this pull request Apr 13, 2023
@shahzadlone
chore: Bump to GoLang v1.19 (#818)
0684901 
- Resolves #817
- Resolves #1070 

- This PR was initially started as: `ci: Bump workflow's golang-ci lint version` to fix the golang-ci lint issue. This work that was done in this PR was the following:
  - Remove the `deadcode` and `varcheck` linters that are now deprecated with `v1.49`
  - Remove all `nolint` directives where the deprecated linters were being referenced.
  - Update CI's linter version to use the latest version.

However as the golang-ci lint team still didn't fix that issue but golang1.19 was released we can just fix those errors and bump the GoLang version for defradb and keeping the previous linter fixes.

Inaddtion also re-enabling the `goheader` linter rule as the change was somehow dropped when we released defradb v0.4.
This was referenced Jul 22, 2023
Closed
Merged
shahzadlone added a commit that referenced this pull request Jul 23, 2023
@shahzadlone
chore: Bump to GoLang v1.20 (#1689)
912824e 
## Relevant issue(s)
- Resolves #522
- Resolves #1687 

## Description
- This is a routine version bump of GoLang, the previous bump was done
in (#818)
- This PR also introduces a new workflow action (not-mandatory to pass
in order to merge) that was showing some vulnerabilities
pre-version-bump, all of the vulnerabilities were resolved once the
golang version was bumped. In future this trigger will be used to bump
golang versions.
- Also updates the golang version for AWS AMI generation.

Note:
- Before the bump we had 13 vulnerabilities:
https://github.com/sourcenetwork/defradb/actions/runs/5629964770/job/15255493129?pr=1688
- After the bump: passing with no vulnerabilities.


## How has this been tested?
- Added action that failed with vulnerabilities.
- Bumped version.
- Vulnerabilities were resolved and action passed.

Specify the platform(s) on which this was tested:
- Arch Linux
shahzadlone added a commit to shahzadlone/defradb that referenced this pull request Feb 23, 2024
@shahzadlone
chore: Bump to GoLang v1.19 (sourcenetwork#818)
d08c38f 
- Resolves sourcenetwork#817
- Resolves sourcenetwork#1070 

- This PR was initially started as: `ci: Bump workflow's golang-ci lint version` to fix the golang-ci lint issue. This work that was done in this PR was the following:
  - Remove the `deadcode` and `varcheck` linters that are now deprecated with `v1.49`
  - Remove all `nolint` directives where the deprecated linters were being referenced.
  - Update CI's linter version to use the latest version.

However as the golang-ci lint team still didn't fix that issue but golang1.19 was released we can just fix those errors and bump the GoLang version for defradb and keeping the previous linter fixes.

Inaddtion also re-enabling the `goheader` linter rule as the change was somehow dropped when we released defradb v0.4.
shahzadlone added a commit to shahzadlone/defradb that referenced this pull request Feb 23, 2024
@shahzadlone
chore: Bump to GoLang v1.20 (sourcenetwork#1689)
6f2f8dd 
## Relevant issue(s)
- Resolves sourcenetwork#522
- Resolves sourcenetwork#1687 

## Description
- This is a routine version bump of GoLang, the previous bump was done
in (sourcenetwork#818)
- This PR also introduces a new workflow action (not-mandatory to pass
in order to merge) that was showing some vulnerabilities
pre-version-bump, all of the vulnerabilities were resolved once the
golang version was bumped. In future this trigger will be used to bump
golang versions.
- Also updates the golang version for AWS AMI generation.

Note:
- Before the bump we had 13 vulnerabilities:
https://github.com/sourcenetwork/defradb/actions/runs/5629964770/job/15255493129?pr=1688
- After the bump: passing with no vulnerabilities.


## How has this been tested?
- Added action that failed with vulnerabilities.
- Bumped version.
- Vulnerabilities were resolved and action passed.

Specify the platform(s) on which this was tested:
- Arch Linux
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@orpheuslummis orpheuslummis orpheuslummis left review comments

@AndrewSisley AndrewSisley AndrewSisley left review comments

@fredcarle fredcarle fredcarle approved these changes

@jsimnz jsimnz Awaiting requested review from jsimnz

Assignees

@shahzadlone shahzadlone

Labels
action/no-benchmark Skips the action that runs the benchmark. blocked bump Bumped version for something deprecate Indicates something is deprecated.
Projects
None yet
Milestone
DefraDB v0.5
Development

Successfully merging this pull request may close these issues.

Bump to GoLang v1.19 as v1.20 is out linters deadcode and varcheck deprecated in golangci-lint v1.49
4 participants
@shahzadlone @orpheuslummis @AndrewSisley @fredcarle