Remove deprecated methods in Options, OptionsBuilder, Attributes & AttributesBuilder

asciidoctor / asciidoctorj

:coffee: Java bindings for Asciidoctor. Asciidoctor on the JVM!

http://asciidoctor.org

Apache License 2.0

617 stars 172 forks source link

Remove deprecated methods in Options, OptionsBuilder, Attributes & AttributesBuilder #1200

Closed abelsromero closed 1 year ago

abelsromero commented 1 year ago

Kind of change

[ ] Bug fix
[ ] New non-breaking feature
[x] New breaking feature
[ ] Documentation update
[ ] Build improvement

Description

What is the goal of this pull request?

See https://github.com/asciidoctor/asciidoctorj/issues/1199

How does it achieve that?

Simply remove methods already marked as deprecated and fix where necessary (mostly tests).

Some extra work removing deprecated use (not the method) of Document::blocks, using now Document::getBlocks

Are there any alternative ways to implement this?

No :thinking: Docs feed from test code, so it will be updated. Still pending a manual review though.

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

It's a breaking change for v3.0.0, nothing to do for v2.5.x branch.

Issue

Fixes #1199

robertpanzer commented 1 year ago

This looks good. Please ping me once you think this is no longer WIP. I'll be off for the next 2 weeks, but I can take a look and merge once it's ready.

abelsromero commented 1 year ago

I'll be off for the next 2 weeks

Enjoy! I need to review the docs and I want to start a "migration guide". We owe to the users to make the migration simple.

I can go back to other projects in the meantime.

abelsromero commented 1 year ago

I decided to also remove AttributesBuilder::setAttributes(Map<String, Object>). I was NOT marked as deprecated, but it's not consistent with the approach of Java-like builders. Users can still use AttributesBuilder::attribute(String) AttributesBuilder::attribute(String, Object) and even ones Attributes instance is created the previous String and Map setters, so no feature is lost for advanced users willing to do more (imho risky) mutable code.

The current WIP migration guide should make things a lot clearer https://github.com/asciidoctor/asciidoctorj/pull/1200/files#diff-a8d0d3d94c218c722c16f7b13a37dc9fc9c477e5a8cb4de289882a848fb4537e.

abelsromero commented 1 year ago

Ready for review :tada:

Squashed changes to keep code and docs changes separated
Relocated to migration guide to be with the other "extension migration guides", I called the new one "API migration guide"
Created adjacent issue to track other deprecated methods to remove for v3.0.0, we are still a few weeks away. That's all I have in mind that we need to v3.0.0 :thinking: (I don't know how many times I thought that already though)