Skip to content

Conversation

@kolyshkin
Copy link
Contributor

@kolyshkin kolyshkin commented Apr 8, 2025

Please see individual commits for details.

(This is based on and includes #10;will rebase if #10 will be merged first).

1. Use tagged switch on value, fixing the staticcheck linter warning:

> fs2/memory.go:22:2: QF1002: could use tagged switch on value (staticcheck)
> 	switch {
> 	^

2. Eliminate named return, and return early, simplifying the code.

Signed-off-by: Kir Kolyshkin <[email protected]>
Ignore this:

> fs2/memory.go:57:7: QF1001: could apply De Morgan's law (staticcheck)
>			if !(errors.Is(err, os.ErrNotExist) && (swapStr == "max" || swapStr == "0")) {
>			   ^

Because it its current form, the if condition corresponds to the
preceding comment.

Signed-off-by: Kir Kolyshkin <[email protected]>
Since commit 8a90b8b the emulator is private, but it is still referred
to as Emulator. Fix this.

Signed-off-by: Kir Kolyshkin <[email protected]>
Both revive and staticcheck complains like this:

> devices/devices_emulator.go:239:20: ST1016: methods on the same type should have the same receiver name (seen 1x "source", 8x "e") (staticcheck)
> func (e *emulator) Apply(rule devices.Rule) error {
>                    ^

What they mean to point to is this line:

> func (source *emulator) Transition(target *emulator) ([]*devices.Rule, error) {

In here, "source" does actually make sense, but instead of suppressing
these warnings, let's just give up and rename the receiver, and
introduce a source variable instead.

Signed-off-by: Kir Kolyshkin <[email protected]>
The new configs were initially created by golangci-lint migrate,
then tailored to simplify and minify.

Signed-off-by: Kir Kolyshkin <[email protected]>
@kolyshkin
Copy link
Contributor Author

I need a second review/LGTM here.
@opencontainers/cgroups-maintainers PTAL

@dims @odinuge @haircommander alas you're not opencontainers.org members so can't be included into the above alias. Nevertheless PTAL

Copy link
Member

@thaJeztah thaJeztah left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@thaJeztah
Copy link
Member

feel free to merge, unless you prefer reviews from the others mentioned

@haircommander
Copy link
Contributor

LGTM

@kolyshkin kolyshkin merged commit 9657f5a into opencontainers:main Apr 24, 2025
13 checks passed
@kolyshkin kolyshkin mentioned this pull request Mar 8, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants