asciidoctor / asciidoctorj

:coffee: Java bindings for Asciidoctor. Asciidoctor on the JVM!
http://asciidoctor.org
Apache License 2.0
623 stars 173 forks source link

Remove invalid destinationDir option from API for v3.0.x #1153

Closed abelsromero closed 1 year ago

abelsromero commented 1 year ago

Kind of change

Description

What is the goal of this pull request?

Fix the issue where using the option destinationDir from API had no effect. The option only is used for the CLI, in that case the value is redirected to 'to_dir'.

How does it achieve that?

  1. Completely removes the option from OptionsBuilder and Options class.
  2. Moves the constant with the name from Options to AsciidoctorUtils: this is the second time I struggle with this class. It's out of place because it contains CLI elements to build the asciidoctor command for logging purposes in the core. So to prevent cyclic dependencies some constants are copied both here and in the asciidoctor-cli sub-module -> Solution = create new sub-module "shared" or remove the logging. Third time will be the charm :thinking:

Are there any alternative ways to implement this?

Initial conversations are about deprecations, but I think it's the moment to do some clean up and breaking changes for v3.0.0.

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

Issue

If this PR fixes an open issue, please add a line of the form:

Fixes #853 Fixes #941

:two: in :one: :grin:

Release notes

Please add a corresponding entry to the file CHANGELOG.adoc

robertpanzer commented 1 year ago

2️⃣ in 1️⃣ 😁

Yep, killed 2 birds with 1 stone :D