Skip to content

Conversation

@frallax
Copy link
Contributor

@frallax frallax commented Mar 13, 2025

What it does

Refactor the InAndOutDataProviderFactory to use the proposed api for configurable data provider factories in org.eclipse.tracecompass PR 235.

Benefit that I think this PR introduces:

  • all configuration logic is contained in the configurable data provider factory (InAndOutDataProviderFactory). This makes very clear how the customization is decided to be implemented (in the case of the InAndOutDataProviderFactory, by creating new analysis modules)
  • the usage of AbstractTmfDataProviderConfigurator "forces" the factories that extend it to have a similar look and feel (apart from sharing some code, easier to test, etc)

How to test

Test will be added if the proposal is accepted.
Checkout org.eclipse.tracecompass PR 235 and incubator PR. Then try to use the InAndOutDataProviderFactory as usual (e.g. by using the tsp-python-client). Can add more explicit instructions if needed.

Follow-ups

Depending on the discussions around this proposal, add documentation on how one can create a new configurable data provider factory using the proposed API.

Review checklist

  • As an author, I have thoroughly tested my changes and carefully followed the instructions in this template

@frallax
Copy link
Contributor Author

frallax commented Mar 13, 2025

NOTES:

  • as a first draft, I am pushing to 2021-06 just because I have a environment configured on that env. It will eventually be pushed to master if discussions go in the right direction
  • please ignore changes to non-java files for the draft PR
  • this PR from org.eclipse.tracecompass is needed to test

Copy link
Contributor

@bhufmann bhufmann left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution. I added some comments,

* the trace to apply it to
* @param writeConfig
* write the config (do only once)
* @return InAndOutAnalysisModule
Copy link
Contributor

Choose a reason for hiding this comment

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

it's a void method

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

* @throws TmfConfigurationException
* if an error occurs
*/
private void create(@NonNull ITmfConfiguration config, @NonNull ITmfTrace trace, boolean writeConfig, IAnalysisModule module) throws TmfConfigurationException {
Copy link
Contributor

Choose a reason for hiding this comment

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

the class is NonNullByDefault, so you can remove the null annotations from the method parameters

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@frallax frallax force-pushed the frallax-config-refactor branch from 9c744a5 to fa57b5d Compare April 3, 2025 12:54
@frallax frallax changed the base branch from 2021-06 to master April 3, 2025 12:54
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