Skip to content

Conversation

@kolyshkin
Copy link
Contributor

@kolyshkin kolyshkin commented Mar 19, 2025

This allows to skip marshalling the default/empty/zero values, thus making the resulting JSON smaller.

While at it, ensure the comment is ending with a comma.

See also: opencontainers/runc#4685


// Uid of the device.
Uid uint32 `json:"uid"`
Uid uint32 `json:"uid,omitempty"` //nolint:revive
Copy link
Member

Choose a reason for hiding this comment

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

Can you add a comment what the nolint was for?

Suggested change
Uid uint32 `json:"uid,omitempty"` //nolint:revive
Uid uint32 `json:"uid,omitempty"` //nolint:revive // Ignore false positive that the pod bay door was open

Copy link
Contributor Author

@kolyshkin kolyshkin Mar 21, 2025

Choose a reason for hiding this comment

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

I actually tried but failed to come up with a nice comment. Something like // Too late to change public API IDs. does not sound good.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Came up with something; PTAL @thaJeztah

This allows to skip marshalling the default/empty/zero values, thus
making the resulting JSON smaller.

While at it,
 - ensure the comment is ending with a comma;
 - add nolint:revive annotations to silence var-naming warnings reported
   by lint-extra (which thinks it's new code). Alas it is too late to
   change those names.

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

@opencontainers/cgroups-maintainers PTAL (I really would like this to be part of runc 1.3)

@kolyshkin kolyshkin requested a review from thaJeztah April 11, 2025 18:18
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

sorry, somehow missed your ping 😓

@thaJeztah thaJeztah merged commit 1d5ac7a into opencontainers:main Apr 12, 2025
13 checks passed
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