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 linter issues in policy.go & acl.go #16366

Merged
merged 5 commits into from
Jul 22, 2022
Merged

Fix linter issues in policy.go & acl.go #16366

merged 5 commits into from
Jul 22, 2022

Conversation

averche
Copy link
Contributor

@averche averche commented Jul 19, 2022

Fixing a few small issues I noticed while looking into policy parameter constraints:

  • nil was passed to context in tests; replaced it with context.Background()
  • t.Fatal was being used inside of internal go-routines, which would not fail the test; changed the test to push errors into a channel and check them at the end
  • use copy and append instead of loops to copy/append slices

@averche averche marked this pull request as draft July 19, 2022 21:50
@averche averche changed the title Averche/policy refator Fix linter issues in policy.go && acl.go Jul 19, 2022
}
}
if pc.DeniedParametersHCL != nil {
pc.Permissions.DeniedParameters = make(map[string][]interface{}, len(pc.DeniedParametersHCL))

for key, val := range pc.DeniedParametersHCL {
pc.Permissions.DeniedParameters[strings.ToLower(key)] = val
for k, v := range pc.DeniedParametersHCL {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

key was shadowing another variable in the scope

@averche averche changed the title Fix linter issues in policy.go && acl.go Fix linter issues in policy.go & acl.go Jul 19, 2022
@averche averche marked this pull request as ready for review July 19, 2022 22:41
@averche averche requested a review from a team July 20, 2022 15:07
Copy link
Contributor

@AnPucel AnPucel left a comment

Choose a reason for hiding this comment

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

Makes sense to me!

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.

None yet

2 participants