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 deprecated methods in Options, OptionsBuilder, Attributes & AttributesBuilder #1200

Conversation

abelsromero
Copy link
Member

@abelsromero abelsromero commented May 9, 2023

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?

See #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 🤔
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

@abelsromero abelsromero force-pushed the issue-1199-remove-deprecated-from-options-and-attributes branch 2 times, most recently from 1efdb03 to 6e2756a Compare May 10, 2023 21:34
@robertpanzer
Copy link
Member

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
Copy link
Member Author

abelsromero commented May 11, 2023

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
Copy link
Member Author

abelsromero commented May 11, 2023

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 abelsromero force-pushed the issue-1199-remove-deprecated-from-options-and-attributes branch 2 times, most recently from ced668a to 4d1ddc7 Compare May 20, 2023 16:59
@abelsromero abelsromero marked this pull request as ready for review May 20, 2023 17:16
@abelsromero
Copy link
Member Author

abelsromero commented May 20, 2023

Ready for review 🎉

  • 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 🤔 (I don't know how many times I thought that already though)
    image

@@ -1,5 +1,6 @@
* Help & Guides
** Updating to New Releases
***xref:api-migration-guide-v25x-to-v30.adoc[]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There seems to be a blank missing here.
Without that the navigation item "Updating to New Releases" is missing, or has the title "v3.0.x migration guide" instead.

I just realized I did not write anything about the changes to the extensions yet. I'll add that once this is merged.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To clarify what I mean, the navigation bar looks like this atm:
Bildschirmfoto 2023-05-29 um 11 24 23

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the fix in the other PR 🙇

Copy link
Member

@robertpanzer robertpanzer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is great! Thank you so much.
I can fix the doc issue in a follow up documenting the changes to the extensions.

@robertpanzer robertpanzer merged commit 1e3aa53 into asciidoctor:main May 29, 2023
@abelsromero abelsromero deleted the issue-1199-remove-deprecated-from-options-and-attributes branch November 26, 2023 10:19
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.

Remove deprecated methods from Options and Attributes
2 participants