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

Remove the module and dynamic cli concepts #464

Merged
merged 8 commits into from
Jan 29, 2025
Merged

Conversation

atenart
Copy link
Contributor

@atenart atenart commented Dec 18, 2024

A high level view is we now have a more manual (but way simpler) way of handling cli arguments and we only have a collectors concept instead of the collectors+modules one. A lot of cleanup was allowed by such changes.

@atenart atenart added the run-functional-tests Request functional tests to be run by CI label Dec 18, 2024
@atenart atenart added this to the v1.6 milestone Dec 18, 2024
@atenart atenart force-pushed the at/module-removal branch 2 times, most recently from 87e49f5 to 0144c64 Compare January 17, 2025 15:19
@atenart
Copy link
Contributor Author

atenart commented Jan 22, 2025

I'm just waiting on #470 to be merged (it should be close) for rebasing this one, to save the other PR a conflict.

@amorenoz
Copy link
Contributor

Changes look good but I guess it's fair to let #470 get merged before?

This is more logical and allows adding collectors to retis/src/collect
in later patches.

Signed-off-by: Antoine Tenart <[email protected]>
This is only moving patches, not changing anything besides the import
mod paths. Following patches will further remove the module concept.

Signed-off-by: Antoine Tenart <[email protected]>
Instead directly use collectors. In details this removes the ModuleId,
Module trait, Modules abstraction and a lot of the dynamic steps that
used to happen when using modules.

Following-up patches are needed to complete to make things completely
clean but those are not strictly about removing the module concept.

Signed-off-by: Antoine Tenart <[email protected]>
This also unties the SectionId <> collector cli arguments relationship.

Signed-off-by: Antoine Tenart <[email protected]>
There is no remaining user after collectors stopped from using it.

Signed-off-by: Antoine Tenart <[email protected]>
Using a more logical order, grouping related parameters and trying to
display then in a logical order (of importance).

Signed-off-by: Antoine Tenart <[email protected]>
@atenart
Copy link
Contributor Author

atenart commented Jan 23, 2025

Rebased on top of main, including #470 which is now merged.

@amorenoz amorenoz merged commit 30ed489 into main Jan 29, 2025
10 of 15 checks passed
@amorenoz amorenoz deleted the at/module-removal branch January 29, 2025 20:12
amorenoz added a commit to amorenoz/retis that referenced this pull request Feb 10, 2025
This is a continuation of discussions that took place during [1].

Some collectors' configuration might affect the way section factories
operate. Pass the section factories to collectors during init() phase so
they can pass such configuration.

The skb collector is the first user of this functionality.

[1] retis-org#464

Signed-off-by: Adrian Moreno <[email protected]>
@amorenoz amorenoz mentioned this pull request Feb 10, 2025
amorenoz added a commit to amorenoz/retis that referenced this pull request Feb 14, 2025
This is a continuation of discussions that took place during [1].

Some collectors' configuration might affect the way section factories
operate. Pass the section factories to collectors during init() phase so
they can pass such configuration.

In order to make use of factories easier, make it a proper type.

The skb collector is the first user of this functionality.

[1] retis-org#464

Signed-off-by: Adrian Moreno <[email protected]>
amorenoz added a commit to amorenoz/retis that referenced this pull request Feb 28, 2025
This is a continuation of discussions that took place during [1].

Some collectors' configuration might affect the way section factories
operate. Pass the section factories to collectors during init() phase so
they can pass such configuration.

In order to make use of factories easier, make it a proper type.

The skb collector is the first user of this functionality.

[1] retis-org#464

Signed-off-by: Adrian Moreno <[email protected]>
amorenoz added a commit to amorenoz/retis that referenced this pull request Feb 28, 2025
This is a continuation of discussions that took place during [1].

Some collectors' configuration might affect the way section factories
operate. Pass the section factories to collectors during init() phase so
they can pass such configuration.

In order to make use of factories easier, make it a proper type.

The skb collector is the first user of this functionality.

[1] retis-org#464

Signed-off-by: Adrian Moreno <[email protected]>
amorenoz added a commit to amorenoz/retis that referenced this pull request Feb 28, 2025
This is a continuation of discussions that took place during [1].

Some collectors' configuration might affect the way section factories
operate. Pass the section factories to collectors during init() phase so
they can pass such configuration.

In order to make use of factories easier, make it a proper type.

The skb collector is the first user of this functionality.

[1] retis-org#464

Signed-off-by: Adrian Moreno <[email protected]>
amorenoz added a commit to amorenoz/retis that referenced this pull request Mar 11, 2025
This is a continuation of discussions that took place during [1].

Some collectors' configuration might affect the way section factories
operate. Pass the section factories to collectors during init() phase so
they can pass such configuration.

In order to make use of factories easier, make it a proper type.

The skb collector is the first user of this functionality.

[1] retis-org#464

Signed-off-by: Adrian Moreno <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
run-functional-tests Request functional tests to be run by CI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants