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

{Dependency,Plugin}UpdatesReport overhaul #637

Closed
jarmoniuk opened this issue Aug 18, 2022 · 4 comments
Closed

{Dependency,Plugin}UpdatesReport overhaul #637

jarmoniuk opened this issue Aug 18, 2022 · 4 comments

Comments

@jarmoniuk
Copy link
Contributor

I see that there's quite some code duplication between DependecyUpdatesReport and PluginUpdatesReport. The classes could possibly use some refactoring. Plus maybe a name change to be in line with the convention (name should probably end with Mojo).

Speaking of which, DependencyUpdatesRenderer uses a completely different interface than DependencyUpdatesXmlRenderer. There's no common factory or a common interface to use the two and the user classes, need to create and handle the two different classes differently.

Here we should create a common API with probably a common abstract factory to instantiate the correct implementation of the renderer.

I appreciate this is not a functional nor a performance issue, but rather a code quality one. I will gladly take it.

@jarmoniuk
Copy link
Contributor Author

Heads up: it looks like using dependency inversion can offer much here, going with it.

@jarmoniuk
Copy link
Contributor Author

Extra item to do: convert xml report generators to "real" ReportGenerators using e.g. the RandomAccessSink or the normal text sink. Then it would be possible to generate those XML reports from <reporting/> too. The execute() invocation would be slightly different, it would simply delegate the generation to two different renderers (with their two different sinks).

@jarmoniuk
Copy link
Contributor Author

Still work in progress, so far have introduced a common model for the renderers and changed renderers into injectable components. It's not done yet -- I'd like to use a templating engine like Velocity. Sadly, it's not possible to inject a "context" into AptParser, otherwise I'd use it and would create a few macros to process content.

For now, refactored and unified the renderers.

master...ajarmoniuk:versions-maven-plugin:doxia-renderers

jarmoniuk added a commit to jarmoniuk/versions-maven-plugin that referenced this issue Oct 6, 2022
jarmoniuk added a commit to jarmoniuk/versions-maven-plugin that referenced this issue Oct 6, 2022
jarmoniuk added a commit to jarmoniuk/versions-maven-plugin that referenced this issue Oct 7, 2022
jarmoniuk added a commit to jarmoniuk/versions-maven-plugin that referenced this issue Oct 7, 2022
jarmoniuk added a commit to jarmoniuk/versions-maven-plugin that referenced this issue Oct 7, 2022
jarmoniuk added a commit to jarmoniuk/versions-maven-plugin that referenced this issue Oct 7, 2022
jarmoniuk added a commit to jarmoniuk/versions-maven-plugin that referenced this issue Oct 8, 2022
@jarmoniuk
Copy link
Contributor Author

Please close @slawekjaranowski @slachiewicz

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