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

Unified parameters for automation and graph formatters #956

Closed
patrickkusebauch opened this issue Aug 12, 2022 · 3 comments
Closed

Unified parameters for automation and graph formatters #956

patrickkusebauch opened this issue Aug 12, 2022 · 3 comments

Comments

@patrickkusebauch
Copy link
Collaborator

patrickkusebauch commented Aug 12, 2022

Right now, the formatter configuration is using as section keys the actual names of the formatters (graphviz, codeclimate). If we change the keys to something more generic (for example graphviz => graph), it would make it reasonable to reuse the same configuration for multiple formatters like #948 or other automation/CI formatters.

That way we also get around the pesky problem of merging those parameters from multiple config files.

@dbrumann
Copy link
Collaborator

dbrumann commented Sep 8, 2022

I have an alternative solution in mind. I want the config to be very specific, instead of having a generic one.

Right now we pass the whole config to each formatter, because I initially wanted to move away from semantic config. Now that we know, it's not easily possible (without breaking merging multiple files) we can make this more specific again.

In the future, I think each formatter should have its dedicated configuration (if the formatter needs it) and extensions can add their own config using PrependExtensionInterface. We would need to make adjustments to register an extension in Deptrac for that, but the foundation is there. If this is too complex, which I can understand, then we can still configure the formatter via service configuration in the deptrac.yaml, i.e. provide the configuration directly to the service instead of via configuration. The downside is, that configuration likely needs to be scalar for this to work easily, but that is the status quo anyways.

@patrickkusebauch
Copy link
Collaborator Author

Well you are the go to Symfony guy for me for development of this repo. So I would defer to your judgement as to what is reasonably simple to implement.

@patrickkusebauch
Copy link
Collaborator Author

Closing as "Won't do"

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

No branches or pull requests

2 participants