-
Notifications
You must be signed in to change notification settings - Fork 247
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 the surrounding loop is unconditionally terminated (SA4004) #1986
Fix the surrounding loop is unconditionally terminated (SA4004) #1986
Conversation
LGTM |
} | ||
return target, nil |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@giuseppe @ktock “Additional layer stores” support multiple stores, but apparently the code was only able to work with the first one, since the very start of the feature ( 64f0181 ).
What do you think about simplifying to only support one store? Admittedly that would not help simplify the codebase all that much, ALS is AFAICS only a feature within this driver and ~this one loop — and instead, we would need to add even more code to reject multiple stores when parsing options.
Either way, we seem not to have tests sufficiently covering the feature.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What do you think about simplifying to only support one store?
SGTM
7afa207
to
72f7e88
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, just a non-blocking stylistic nit.
Please squash follow-on fixes instead of leaving the original versions of commits in the PR in the history.
72f7e88
to
34832d4
Compare
Signed-off-by: Jan Rodák <[email protected]>
34832d4
to
599c5d0
Compare
/approve |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: Honny1, rhatdan The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
/lgtm |
This PR fixes
the surrounding loop is unconditionally terminated
warning (SA4004) found bygolangci
when thestaticcheck
linter is enabled.Partially fixes: