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

Update GO v1.21 #476

Closed
wants to merge 4 commits into from
Closed

Conversation

mpanchajanya
Copy link
Contributor

@mpanchajanya mpanchajanya commented Sep 11, 2023

What this PR does / why we need it

  • Update go.mod to point to go v1.21
  • Update tools like linters to latest
  • Fix lint issues

Which issue(s) this PR fixes

Fixes #

Describe testing done for PR

  • Build the Tanzu CLI
  • Perform basic lifecycle ops
  • Unit tests, Lint checks

Release note

Tanzu CLI is updated to latest go v1.21

Additional information

Special notes for your reviewer

@mpanchajanya mpanchajanya self-assigned this Sep 11, 2023
@mpanchajanya mpanchajanya marked this pull request as ready for review September 11, 2023 20:32
@mpanchajanya mpanchajanya requested a review from a team as a code owner September 11, 2023 20:32
@mpanchajanya mpanchajanya changed the title Update Go v1.21 Update GO v1.21 Sep 11, 2023
- EXC0011 # disable excluding of issues about missing package comments from stylecheck

# Show all errors for all linters. Setting these to 0 disables limiting error reporting.
- EXC0011
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we keep the comment as it is.

Suggested change
- EXC0011
- EXC0011 # disable excluding of issues about missing package comments from stylecheck

Comment on lines -78 to -84
- varcheck
- whitespace
- bodyclose
- noctx
- rowserrcheck
- structcheck
- unparam
Copy link
Contributor

Choose a reason for hiding this comment

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

Are these removals intentional?

Copy link
Contributor

Choose a reason for hiding this comment

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

same question. If they are, in the future it would be good to mention in the PR description or a comment in this file

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes these are deprecated in new golangci-lint version. Updated the PR description

@@ -96,6 +96,7 @@ func runLocalListener() error {
mux := http.NewServeMux()
mux.HandleFunc("/callback", callbackHandler)
tokenExchange, tokenExchangeComplete = context.WithCancel(context.TODO())
//nolint:gosec
Copy link
Contributor

Choose a reason for hiding this comment

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

If possible might be better to add a comment on why we are skipping this.

if hdr.Typeflag == tar.TypeReg || hdr.Typeflag == tar.TypeRegA {
if hdr.Typeflag == tar.TypeReg {
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this change intentional?

Comment on lines -78 to -84
- varcheck
- whitespace
- bodyclose
- noctx
- rowserrcheck
- structcheck
- unparam
Copy link
Contributor

Choose a reason for hiding this comment

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

same question. If they are, in the future it would be good to mention in the PR description or a comment in this file

- "!$test"
allow:
- $gostd
- github.com/AlecAivazis/survey/v2
Copy link
Contributor

Choose a reason for hiding this comment

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

this sounds like sounding that is rather cumbersome to maintain. Can you summarize when a pattern needs to be added to the allow list, because it does not seem to cover all of go.mod

@mpanchajanya
Copy link
Contributor Author

Addressing the comments as part of #490

@mpanchajanya mpanchajanya deleted the go-update branch November 16, 2023 00:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants