Skip to content

Issues/do not inherit pseudoplugins #1801

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

Open
wants to merge 3 commits into
base: develop
Choose a base branch
from

Conversation

ikelos
Copy link
Member

@ikelos ikelos commented May 8, 2025

Ok, this aims to remove any traces of inheritance or pseudo-inheritance, by using only modular classmethods and ensuring each component lists its requirements (and the requirement checking should ensure everything can operate correctly).

Making this change pointed out that lsmod was using modules that were pulled in only by the "module display plugin" functionality. Each plugin now lists exactly what it requires without trying to pull it in from the module. This also improves a number of typing inaccuracies that had been present.

@ikelos ikelos requested a review from atcuno May 8, 2025 13:40
@atcuno
Copy link
Contributor

atcuno commented May 8, 2025

What does this mean?:

'lsmod was using modules that were pulled in only by the "module display plugin"'

Is there a ticket or Slack discussion about whatever triggered this PR? I want to understand what went wrong to avoid in future code.

@ikelos
Copy link
Member Author

ikelos commented May 8, 2025

Yeah, it's all in #1773.

lsmod was using modules that were pulled in only by the "module display plugin"

lsmod used some requirements in its code that it relied on coming from the + linux_utilities_module_display_plugin.get_requirements() that was being added to lsmod's requirements. This meant that it couldn't stand alone without that. It's all fixed now by adding the specific requirement, it just shows that trying to blend two bits of code into a single plugin muddies which bits of code do what.

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.

2 participants