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: Upgrade Golang version from 1.18 to 1.19 #28

Merged
merged 4 commits into from
Nov 30, 2023

Conversation

spolti
Copy link
Contributor

@spolti spolti commented Oct 26, 2023

Fixed lint issues:

Can't run linter goanalysis_metalinter: goanalysis_metalinter: buildir: package "netip" (isInitialPkg: false, needAnalyzeSource: true): in net/netip.AddrFromSlice: cannot convert Load <[]byte> t0 ([]byte) to [4]byte

Warnings:

WARN [runner] The linter 'structcheck' is deprecated (since v1.49.0) due to: The owner seems to have abandoned the linter.  Replaced by unused.
WARN [runner] The linter 'deadcode' is deprecated (since v1.49.0) due to: The owner seems to have abandoned the linter.  Replaced by unused.
WARN [runner] The linter 'varcheck' is deprecated (since v1.49.0) due to: The owner seems to have abandoned the linter.  Replaced by unused.

@ckadner ckadner marked this pull request as draft October 28, 2023 07:44
@ckadner
Copy link
Member

ckadner commented Oct 28, 2023

Hi @spolti -- could you separate the changes into 2 PRs -- without RHODS or CVE numbers in the title ;-)

  • "chore: Upgrade Go from 1.18 to 1.19"
  • "chore(deps): Upgrade golang.org/x/net and golang.org/grpc"

@spolti
Copy link
Contributor Author

spolti commented Oct 31, 2023

Hi @ckadner, sure.

@spolti spolti force-pushed the CVE-2023-44487 branch 3 times, most recently from ccf4a0e to 0defb5d Compare October 31, 2023 12:52
@spolti spolti marked this pull request as ready for review October 31, 2023 12:52
@spolti
Copy link
Contributor Author

spolti commented Oct 31, 2023

Done, dependencies upgrade: #30

@ckadner I don't have permissions to see the dependabot issues, are they still related only with the Go lang update? Or should we update the above PR's descrioption?

@spolti spolti changed the title [RHODS-12555] - CVE-2023-44487 CVE-2023-44487 Oct 31, 2023
@ckadner ckadner changed the title CVE-2023-44487 chore: Upgrade Golang version from 1.17 to 1.19 Nov 14, 2023
@ckadner ckadner changed the title chore: Upgrade Golang version from 1.17 to 1.19 chore: Upgrade Golang version from 1.18 to 1.19 Nov 14, 2023
Copy link
Member

@ckadner ckadner left a comment

Choose a reason for hiding this comment

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

Thanks @spolti -- My apologies for the delayed review.

Dockerfile Outdated Show resolved Hide resolved
go.mod Show resolved Hide resolved
scripts/fmt.sh Outdated Show resolved Hide resolved
scripts/fmt.sh Outdated Show resolved Hide resolved
@spolti
Copy link
Contributor Author

spolti commented Nov 16, 2023

the CI issue will be fixed by #30

@spolti spolti force-pushed the CVE-2023-44487 branch 2 times, most recently from 76475fc to f2f6ca2 Compare November 21, 2023 19:37
@spolti spolti requested a review from ckadner November 22, 2023 13:57
@ckadner
Copy link
Member

ckadner commented Nov 23, 2023

Hi @spolti -- I added another comment on a thread from my past review. Other than that, your PR looks good 👍🏻

Copy link
Member

@ckadner ckadner left a comment

Choose a reason for hiding this comment

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

Thanks @spolti -- your changes look good. Just need to update the .pre-commit.log configuration. See comment above.

@spolti
Copy link
Contributor Author

spolti commented Nov 23, 2023

Thanks @ckadner, sry for the late reply, busy day.

@ckadner ckadner added this to the v0.11.2 milestone Nov 23, 2023
chore: Upgrade Go from 1.18 to 1.19

Plus:

Fixes lint issues:

- Can't run linter goanalysis_metalinter: goanalysis_metalinter: buildir: package "netip" (isInitialPkg: false, needAnalyzeSource: true): in net/netip.AddrFromSlice: cannot convert Load <[]byte> t0 ([]byte) to [4]byte

Warnings:

WARN [runner] The linter 'structcheck' is deprecated (since v1.49.0) due to: The owner seems to have abandoned the linter.  Replaced by unused.
WARN [runner] The linter 'deadcode' is deprecated (since v1.49.0) due to: The owner seems to have abandoned the linter.  Replaced by unused.
WARN [runner] The linter 'varcheck' is deprecated (since v1.49.0) due to: The owner seems to have abandoned the linter.  Replaced by unused.

Signed-off-by: Spolti <[email protected]>
Signed-off-by: Spolti <[email protected]>
Copy link
Member

@rafvasq rafvasq left a comment

Choose a reason for hiding this comment

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

@ckadner, I tested this change too with no issues. Similar to kserve/modelmesh-runtime-adapter#68 (comment):

I deployed MM with your updates to this component and deployed/inferenced an inference service as outlined in the quickstart guide :)

Copy link
Member

@ckadner ckadner left a comment

Choose a reason for hiding this comment

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

Thanks @spolti -- just a small nit pick

.gitignore Outdated Show resolved Hide resolved
Copy link
Member

@ckadner ckadner left a comment

Choose a reason for hiding this comment

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

Great! Thanks @spolti

/lgtm
/approve

Copy link

oss-prow-bot bot commented Nov 30, 2023

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: ckadner, spolti

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@ckadner ckadner merged commit 61dc700 into kserve:main Nov 30, 2023
5 of 6 checks passed
@spolti spolti deleted the CVE-2023-44487 branch November 30, 2023 17:45
spolti referenced this pull request in spolti/rest-proxy Jan 10, 2024
- Remove the linters for "deadcode", "structcheck", "varcheck"
- Use "os" packages instead of deprecated "io/ioutil" (SA1019)
- Capture pre-commit output in a local log file

---------

Signed-off-by: Spolti <[email protected]>
spolti referenced this pull request in spolti/rest-proxy Jan 10, 2024
- Remove the linters for "deadcode", "structcheck", "varcheck"
- Use "os" packages instead of deprecated "io/ioutil" (SA1019)
- Capture pre-commit output in a local log file

---------

Signed-off-by: Spolti <[email protected]>
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.

3 participants