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 lint issues in util #183

Merged
merged 1 commit into from
Nov 4, 2023
Merged

Conversation

nickajacks1
Copy link
Contributor

Also reduce execution time of ConvertComponentIdIntoFriendlyPathSearch by 50-60% and add benchmark

Lint issues fixed:

utils/utils.go:567:22: S1007: should use raw string (`...`) with regexp.MustCompile to avoid having to escape twice (gosimple)
var bracketNameExp = regexp.MustCompile("^(\\w+)\\[(\\w+)\\]$")
                     ^
utils/utils_test.go:232:28: S1040: type assertion to the same type: parsed already has type interface{} (gosimple)
        assert.Equal(t, "niblet", parsed.(interface{}))
                                  ^
utils/utils_test.go:171:2: S1021: should merge variable declaration with assignment on next line (gosimple)
        var d interface{}
        ^
utils/utils_test.go:198:2: S1021: should merge variable declaration with assignment on next line (gosimple)
        var d interface{}
        ^
utils/utils_test.go:236:2: S1021: should merge variable declaration with assignment on next line (gosimple)
        var d interface{}
        ^
utils/utils.go:615:33: S1025: the argument is already a string, there's no need to use fmt.Sprintf (gosimple)
                replaced = strings.ReplaceAll(fmt.Sprintf("%s",
                                              ^
utils/utils.go:618:33: S1025: the argument is already a string, there's no need to use fmt.Sprintf (gosimple)
                replaced = strings.ReplaceAll(fmt.Sprintf("%s",
                                              ^
utils/utils.go:576:21: SA6000: calling regexp.MatchString in a loop has poor performance, consider using regexp.Compile (staticcheck)                pathCharExp, _ := regexp.MatchString("[%=;~.]", segs[i])
                                  ^

Reduce execution time of ConvertComponentIdIntoFriendlyPathSearch by
50-60% and add benchmark

Signed-off-by: Nicholas Jackson <[email protected]>
@codecov
Copy link

codecov bot commented Oct 10, 2023

Codecov Report

All modified lines are covered by tests ✅

Comparison is base (1d566cd) 99.75% compared to head (84c9447) 99.75%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #183      +/-   ##
==========================================
- Coverage   99.75%   99.75%   -0.01%     
==========================================
  Files         149      149              
  Lines       10822    10821       -1     
==========================================
- Hits        10796    10794       -2     
- Misses         26       27       +1     
Flag Coverage Δ
unittests 99.75% <100.00%> (-0.01%) ⬇️

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

Files Coverage Δ
utils/utils.go 100.00% <100.00%> (ø)

... and 1 file with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@nickajacks1
Copy link
Contributor Author

Hmmm why did coverage go down on an unrelated file? I'll take a look later...

@@ -573,8 +574,7 @@ func ConvertComponentIdIntoFriendlyPathSearch(id string) (string, string) {

// check for strange spaces, chars and if found, wrap them up, clean them and create a new cleaned path.
for i := range segs {
pathCharExp, _ := regexp.MatchString("[%=;~.]", segs[i])
if pathCharExp {
if pathCharExp.Match([]byte(segs[i])) {
Copy link
Member

Choose a reason for hiding this comment

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

Nice! yeah all these regexes should always be pre-compiled, I must have been rushing.

Copy link
Member

@daveshanley daveshanley left a comment

Choose a reason for hiding this comment

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

LGTM.

@daveshanley
Copy link
Member

I ignore any off by one code coverage results 0.01% is a rounding error in coverage and not a concern of mine.

@daveshanley daveshanley merged commit def8e99 into pb33f:main Nov 4, 2023
2 of 3 checks passed
@nickajacks1 nickajacks1 deleted the utils-gosimple branch November 4, 2023 16:01
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