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

Fix remaining lint errors #578

Merged
merged 1 commit into from
May 30, 2023
Merged

Fix remaining lint errors #578

merged 1 commit into from
May 30, 2023

Conversation

r-hang
Copy link
Contributor

@r-hang r-hang commented May 30, 2023

After rebasing #574 on top of #577 and running GO111MODULE=on make lint I still see some errors.

cmd/thriftbreak/main_test.go:26:2: "io/ioutil" has been deprecated since Go 1.19: As of Go 1.16, the same functionality is now provided by package io or package os, and those implementations should be preferred in new code. See the specific function documentation for details. (SA1019)
gen/string.go:66:15: strings.Title has been deprecated since Go 1.18 and an alternative has been available since Go 1.0: The rule Title uses for word boundaries does not handle Unicode punctuation properly. Use golang.org/x/text/cases instead. (SA1019)

After this change:

$ git log --oneline
a054ce6 (HEAD -> optimise_read_string) Fix remaining lint errors
2c1d285 build tags
8a1aa7d address review comments
f26170a inline
41dacbd remove bench reports
4ad4d9c optimise write string
fb22d6e optimise readstring()
6268dfe (origin/dev, origin/HEAD) Remove all usages of io/ioutil (#577)

$ GO111MODULE=on make lint
Checking gofmt
Checking govet
Checking golint
Checking staticcheck
$ echo $?
0

I've cherry-picked the contents of the HEAD commit displayed above into a separate branch for a PR to dev.

@r-hang r-hang requested a review from sywhang May 30, 2023 23:22
@codecov
Copy link

codecov bot commented May 30, 2023

Codecov Report

Merging #578 (86d21fa) into dev (6268dfe) will not change coverage.
The diff coverage is 100.00%.

❗ Current head 86d21fa differs from pull request most recent head c7a5992. Consider uploading reports for the commit c7a5992 to get more accurate results

@@           Coverage Diff           @@
##              dev     #578   +/-   ##
=======================================
  Coverage   68.00%   68.00%           
=======================================
  Files         140      140           
  Lines       23872    23872           
=======================================
  Hits        16233    16233           
  Misses       4578     4578           
  Partials     3061     3061           
Impacted Files Coverage Δ
gen/string.go 78.57% <100.00%> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

After rebasing #574 on top of #577 and running GO111MODULE=on make lint
I still see some errors.

cmd/thriftbreak/main_test.go:26:2: "io/ioutil" has been deprecated since Go 1.19: As of Go 1.16, the same functionality is now provided by package io or package os, and those implementations should be preferred in new code. See the specific function documentation for details.  (SA1019)
gen/string.go:66:15: strings.Title has been deprecated since Go 1.18 and an alternative has been available since Go 1.0: The rule Title uses for word boundaries does not handle Unicode punctuation properly. Use golang.org/x/text/cases instead.  (SA1019)

After this change:

$ git log --oneline
a054ce6 (HEAD -> optimise_read_string) Fix remaining lint errors
2c1d285 build tags
8a1aa7d address review comments
f26170a inline
41dacbd remove bench reports
4ad4d9c optimise write string
fb22d6e optimise readstring()
6268dfe (origin/dev, origin/HEAD) Remove all usages of io/ioutil (#577)

$ GO111MODULE=on make lint
Checking gofmt
Checking govet
Checking golint
Checking staticcheck
$ echo $?
0
@r-hang r-hang force-pushed the rhang/fix-remaining-lint branch from 86d21fa to c7a5992 Compare May 30, 2023 23:29
@r-hang r-hang merged commit 1e96648 into dev May 30, 2023
@r-hang r-hang deleted the rhang/fix-remaining-lint branch May 30, 2023 23:35
r-hang added a commit that referenced this pull request Jun 9, 2023
PR #578 replace strings.Title with golang.org/x/text/cases due to
staticcheck SA1019. There is at least one behavioral difference
between these two Title casing approaches uncovered during pre-release
testing.

A test case to flag this change and an exemption for staticcheck in
the relevant file are also added.
r-hang added a commit that referenced this pull request Jun 9, 2023
PR #578 replace strings.Title with golang.org/x/text/cases due to
staticcheck SA1019. There is at least one behavioral difference
between these two Title casing approaches uncovered during pre-release
testing.

A test case to flag this change and an exemption for staticcheck in
the relevant file are also added.

Example:
chunk := "10x"
fmt.Println(strings.Title(strings.ToLower(chunk))) # 10x
fmt.Println(cases.Title(language.English).String(strings.ToLower(chunk))) # 10X
r-hang added a commit that referenced this pull request Jun 9, 2023
PR #578 replace strings.Title with golang.org/x/text/cases due to
staticcheck SA1019. There is at least one behavioral difference
between these two Title casing approaches uncovered during pre-release
testing.

A test case to flag this change and an exemption for staticcheck in
the relevant file are also added.

Example:
```
chunk := "10x"
fmt.Println(strings.Title(strings.ToLower(chunk))) # 10x
fmt.Println(cases.Title(language.English).String(strings.ToLower(chunk))) # 10X
```
r-hang added a commit that referenced this pull request Jun 9, 2023
PR #578 replaced strings.Title with golang.org/x/text/cases due to
staticcheck SA1019. There is at least one behavioral difference
between these two Title casing approaches uncovered during pre-release
testing.

A test case to flag this change and an exemption for staticcheck in
the relevant file are also added.

Example:
```
chunk := "10x"
fmt.Println(strings.Title(strings.ToLower(chunk))) # 10x
fmt.Println(cases.Title(language.English).String(strings.ToLower(chunk))) # 10X
```
r-hang added a commit that referenced this pull request Jun 9, 2023
PR #578 replaced strings.Title with golang.org/x/text/cases due to
staticcheck SA1019. There is at least one behavioral difference
between these two Title casing approaches uncovered during pre-release
testing.

A test case to flag this change and an exemption for staticcheck in
the relevant file are also added.

Example:
```
chunk := "10x"
fmt.Println(strings.Title(strings.ToLower(chunk))) # 10x
fmt.Println(cases.Title(language.English).String(strings.ToLower(chunk))) # 10X
```
r-hang added a commit that referenced this pull request Jun 9, 2023
PR #578 replaced strings.Title with golang.org/x/text/cases due to
staticcheck SA1019. There is at least one behavioral difference
between these two Title casing approaches uncovered during pre-release
testing.

A test case to flag this change and an exemption for staticcheck in
the relevant file are also added.

Example:
```
chunk := "10x"
fmt.Println(strings.Title(strings.ToLower(chunk))) # 10x
fmt.Println(cases.Title(language.English).String(strings.ToLower(chunk))) # 10X
```
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.

2 participants