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

Reimplemented resource copy using 'plexus.util.DirectorScanner' instead of maven-filtering #597

Merged

Conversation

abelsromero
Copy link
Member

In exchange for some extra file management code, this reduces the amount of dependencies
used and injected into AsciidoctorMojo. It should also speed up build times since we
are doing much less work now to copy resources.

  • Created model classes for Resources to ensure Mojo descriptors don't leak internals
  • Remove 'org.apache.maven.shared:maven-filtering' dependency
  • Bump plexus-utils to v3.3.1
  • Remove unnecessary IT 'resource-filtering' since "under-the-hood" filtering is no longer possible

closes #335

What kind of change does this PR introduce? (check at least one)

  • Bugfix
  • Feature
  • Documentation
  • Refactor
  • Build improvement
  • Other (please describe)

What is the goal of this pull request?

  • Simplify code, reduce dependencies (libraries and runtime components injected in AsciidoctorMojo).
  • Disambiguate "maven filtering" (aka. content replacement) features that where nos supported but available if users knew the internals.

Are there any alternative ways to implement this?
We could have have just exposed a custom model pojo to avoid leaking filtering. But I think is worth the effort given the simplification of dependencies and code.

Are there any implications of this pull request? Anything a user must know?
No.

Is it related to an existing issue?

  • Yes
  • No

Finally, please add a corresponding entry to CHANGELOG.adoc

…Scanner'

 instead of maven-filtering

In exchange for some extra file management code, this reduces the amount of dependencies
used and injected into AsciidoctorMojo. It should also speed up build times since we
are doing much less work now to copy resources.

* Created model classes for Resources to ensure Mojo descriptors don't leak internals
* Remove 'org.apache.maven.shared:maven-filtering' dependency
* Bump plexus-utils to v3.3.1
* Remove unnecessary IT 'resource-filtering' since "under-the-hood" filtering is no longer possible
@abelsromero
Copy link
Member Author

First pass implementation based on passing current tests, but I want to do some refactor and create a component for resource management that can be unit tested.

@abelsromero abelsromero force-pushed the issue-335-remove-maven-filtering branch 3 times, most recently from bf35d10 to 638923c Compare August 27, 2022 18:44
* Remove unused encoding from resource processing
* Created CopyResourcesProcessor
* ResourcesProcessor no longer throws MojoExecutionException
@abelsromero abelsromero force-pushed the issue-335-remove-maven-filtering branch from 638923c to f2cdb1e Compare August 27, 2022 20:37
@abelsromero
Copy link
Member Author

Run some tests with a project with 20k resources and the time reduction at the end is marginal.

@abelsromero abelsromero marked this pull request as ready for review August 27, 2022 21:11
@abelsromero abelsromero merged commit 1f9bfe3 into asciidoctor:main Aug 27, 2022
abelsromero added a commit to abelsromero/asciidoctor-maven-plugin that referenced this pull request Aug 28, 2022
@abelsromero abelsromero deleted the issue-335-remove-maven-filtering branch December 28, 2023 20:18
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.

Consider DirectoryScanner as alternative to copy resources using maven-filtering
1 participant