Skip to content

Conversation

@milkpirate
Copy link

No description provided.

Signed-off-by: Paul Schroeder <[email protected]>
@CLAassistant
Copy link

CLAassistant commented Jun 2, 2025

CLA assistant check
All committers have signed the CLA.

@sagikazarmark
Copy link
Collaborator

Thanks for the PR.

I'm not sure that's enough though, because the err variable gets shadowed on the second line of the function.

I think the appropriate solution is dropping the variable names from the return declaration.

@milkpirate
Copy link
Author

  • Not really, the := in line 77 is necessary to declare fi, it will set the predeclared err as well. No shadowing (not like in L94).
  • I did not remove the named returns, since if one does so, one has to explicitly return the corresponding err/nils as well as the actual values ([]string). I wanted to keep the impact minimal to increase the chance of getting the PR merged 😄

glob tests are fine with the changes:

=== RUN   TestGlob
    tarfs_test.go:408: glob: \*: glob ok
    tarfs_test.go:408: glob: *: glob ok
    tarfs_test.go:408: glob: sub\*: glob ok
    tarfs_test.go:408: glob: sub\testDir2\*: glob ok
    tarfs_test.go:408: glob: testDir1\*: glob ok
--- PASS: TestGlob (0.00s)

just not sure if they cover all cases.

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.

3 participants