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

Minor cleanups in libcontainer and loadFactory #3340

Conversation

thaJeztah
Copy link
Member

libct.IntelRdtFs(): simplify

  • remove unneeded wrapping func, as intelrdt.NewManager already has the correct signature.
  • reverse the if/else to be clearer that either "IsCATEnabled()" and/or "IsMBAEnabled()" need to be enabled (and to allow skipping one of those checks in the condition).

loadFactory(): remove unneeded intermediate variable

- remove unneeded wrapping func, as intelrdt.NewManager already has the
  correct signature.
- reverse the if/else to be clearer that either "IsCATEnabled()" and/or
  "IsMBAEnabled()" need to be enabled (and to allow skipping one of those
  checks in the condition).

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
@thaJeztah thaJeztah requested a review from kolyshkin January 19, 2022 10:30
if !intelrdt.IsCATEnabled() && !intelrdt.IsMBAEnabled() {
l.NewIntelRdtManager = nil
if intelrdt.IsCATEnabled() || intelrdt.IsMBAEnabled() {
l.NewIntelRdtManager = intelrdt.NewManager
Copy link
Member Author

Choose a reason for hiding this comment

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

FWIW; wondering why NewIntelRdtManager (and other fields in LinuxFactory are exported; perhaps they should not be)

@kolyshkin
Copy link
Contributor

Can we please have this one after #3316? That one is a major refactoring and is removing some stuff which is modified here.

@thaJeztah
Copy link
Member Author

Yes! Fine to do it after, if anything is left then (I didn't see those changes until after I created this PR ☺️)

@kolyshkin kolyshkin marked this pull request as draft January 27, 2022 19:17
@kolyshkin
Copy link
Contributor

It looks like there's nothing left to fix after #3354 (commit dbd990d)

@kolyshkin kolyshkin closed this Aug 16, 2022
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.

2 participants