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

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

Conversation

abelsromero
Copy link
Member

Kind of change

  • Bug fix
  • New non-breaking feature
  • New breaking feature
  • Documentation update
  • Build improvement

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 🤔

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

2️⃣ in 1️⃣ 😁

Release notes

Please add a corresponding entry to the file CHANGELOG.adoc

@abelsromero abelsromero force-pushed the issue-853-remove-invalid-destinationDir-option branch 2 times, most recently from 9da0c8c to 369acd8 Compare March 30, 2023 20:49
The option only is used for the CLI, in that case
the value is redirected to 'to_dir'.

Fixes asciidoctor#853
Fixes asciidoctor#941
@abelsromero abelsromero changed the title Remove invalid destinationDir option from API Remove invalid destinationDir option from API for v3.0.x Mar 30, 2023
@robertpanzer robertpanzer merged commit e472e42 into asciidoctor:main Mar 31, 2023
@robertpanzer
Copy link
Member

2️⃣ in 1️⃣ 😁

Yep, killed 2 birds with 1 stone :D

@abelsromero abelsromero deleted the issue-853-remove-invalid-destinationDir-option branch April 21, 2023 08:25
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.

Deprecation of Options destinationDir Destination directory ignored in Asciidoctorj
2 participants