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

release 3.0.0-alpha.1 is gravely incompatible with earlier releases #1220

Closed
verhas opened this issue Jun 16, 2023 · 11 comments
Closed

release 3.0.0-alpha.1 is gravely incompatible with earlier releases #1220

verhas opened this issue Jun 16, 2023 · 11 comments

Comments

@verhas
Copy link

verhas commented Jun 16, 2023

Preprocessors are required to implement the org.asciidoctor.extension.Preprocessor abstract class.
From version 2.5.8 version to 3.0.0-alpha-1 the method signature of

public abstract void process(Document document, PreprocessorReader reader);

changed to

public abstract Reader process(Document document, PreprocessorReader reader);

A preprocessor cannot implement both incompatible methods. When I develop the Jamal preprocessor Asciidoctor integration, I either go with the 2.5.8 version and support all those implementations that use that version (e.g., the IntelliJ Asciidoctor plugin as of today, 2023-06-16). My software starts not working when the plugin upgrade uses the new version. Or I opt for the new, unusable version until the new plugin version comes out.

However, when the new version is out, I will have users who did NOT upgrade to the newest version and users who did.

Or I invest a great amount of technical development and create a code that makes serious class loading tricks, decides on the fly during run-time which version is needed, and registers the one which fits. It is doable but hacky, adding an extra reflective call overhead for each render.

How can you fix this? Can you revert this release and have a version that keeps compatibility with the API interface?

Also, it would be nice to see how you decide on such a compatibility-breaking change before it gets into release and makes extension developers' life a nightmare.

@verhas
Copy link
Author

verhas commented Jun 16, 2023

@abelsromero
Copy link
Member

abelsromero commented Jun 16, 2023

Thanks for the feedback! But I must say that this is a major version and it has been part of its goals to finally implement some improvements that break backward compatibility. As with any piece of software, that's bound to happen at some point. We are aware the changes won't be "nice" and that's why an alpha release to allow users to start testing was released. We hope we can catch bugs and improve upon the work done, but we cannot maintain interfaces that limited the functionality.
Branch v2.5.x will still be supported for some time, so you don't need to upgrade as soon as possible. I can't comment on the other modules like IntelliJ plugin. If you integrate with that too, you could try to reach the maintainer about a possible calendar, https://asciidoctor.zulipchat.com would be the best place. The truth is we don't have any closed date for the final release, but we could coordinate with him and others if it helps the community.

If it's within your grasp I can suggest using a branching strategy to start working on v3.0.0. We are doing it for the maven-plugin. That helps keep things separated, at the cost of some duplication of course.

PS: we have prepared special "v25x to v30" documentation to facilitate the migration https://github.com/asciidoctor/asciidoctorj/tree/main/docs/modules/guides/pages.

@verhas
Copy link
Author

verhas commented Jun 17, 2023

Thanks for coming back to me that fast. I appreciate.

I understand that significant versions break backward compatibility. I was reading the old comments why this method was void in the first place, and I can tell, IMHO, it is absolutely reasonable to change the API this direction. The direction is okay, the way, however, could be better paved.

I suggest creating the new SPI in a way that leaves room for implementing a preprocessor supporting both versions. I would like to create a preprocessor that can work with the 2.X.X and the 3.X.X versions simultaneously, if possible.

You could, for example, rename the method for the 3.X.X versions and deprecate or even delete the old one. The implementation can implement both methods.

Your approach makes it challenging to create an implementation supporting both versions. It is not impossible because Java is highly versatile in this sense. However, I had to create a new module in my project with the dependencies on the 2.5.10 version and implement a code, which determines which AsciidoctorJ version is calling it.

It is a technical bravado, I can write an article about it, and I enjoyed the two hours of writing and debugging this hack. Now it is ready, so Jamal supports 2.X.X and 3.X.X versions simultaneously as of its next release 2.3.0.

However, making your fellow developers' life easy is in your best interest. Preprocessor developers want to focus on the functionality of their software and not on compatibility upgrade changes. I know; I am one. Creating different releases and different branches is possible, but it has its cost. It is not your cost, but still something that we can save.

@verhas verhas closed this as completed Jun 17, 2023
@verhas
Copy link
Author

verhas commented Jun 17, 2023

BTW: the document https://github.com/asciidoctor/asciidoctorj/blob/main/docs/modules/guides/pages/api-migration-guide-v25x-to-v30.adoc is not there. I mean, it is there, listing other documents, which all return 404.

@abelsromero
Copy link
Member

First, these are MY comments, I am one of the maintainers and I think I can speak for the whole, but the final decision is open to discussion with the rest. In that regard, can you please update your doc to refer to AsciidoctorJ developers? You are entitled to your opinions, but please don't extend them to others, Asciidoctor refers to the Ruby core implementation maintained by other members of the community.


I am glad you could find a solution.
I must say I don't consider the complexity of an argument in this kind of conversation. It is a decision for the integrator to decide what versions to support and how to do it. That should not impose restrictions on the consumed libraries.
Believe it or not, maintaining multiple products (repos) with multiple release versions is part of my day-to-day job (between 5 and 3 release branches for each repo). Even more in asciidoctor-maven-plugin we maintain also several branches based on AsciidoctorJ, maven-site v4-milestones, and so far we are lucky we don't need them for Maven v4-milestones 🤞
Believe me when I say I understand your pain, but precisely I clearly see the need for libraries to be ale to evolve independently.

But you brought up some fair points:

  • Extension changes have not been marked as deprecated v2.5.x. So users haven't received a warning about it. I think, we address that with the alpha and maintain v2.5.x branch (time TBD) for some time after v3.0.x release. If we decide to support both interfaces, it would be in a future v2.5.x, not in v3.0.x. And even if we did in v3, they will be removed in v4 eventually.
  • You are right the links from GH don't work. Docs are built with Antora, you can find the included at https://github.com/asciidoctor/asciidoctorj/tree/main/docs/modules/guides/partials, in the meantime, I opened a thread in Zulip to discuss how to publish them (seems we can do it nicely). Definitely pointing to the sources in GH is not a good way, sorry. We'll address that, thanks.

@mojavelinux
Copy link
Member

mojavelinux commented Jun 18, 2023

As an observer, I'd like to point out that I find this comment in the referenced doc to be extremely unfriendly and uncalled for:

This module is a patch to the terrible decision of the Asciidoctor developers to change the SPI in an incompatible way.

First, you're referring to an alpha. Everyone understands that an alpha is a proposal. So to say that a decision was made is completely inaccurate. A decision has been proposed at best. And you seem to have written that accusation before you even brought the issue to the attention of the project and provided ample time for a response. It's entirely possible that the consequences of such a proposal weren't even understood at the time.

In short, be nice. And I don't find that comment to be nice. We're all trying to create great software and we don't get there by shaming others. That's not how open source works when it works.

@verhas
Copy link
Author

verhas commented Jun 18, 2023

Dear Abel,

Believe it or not, ... is part of my day-to-day job

That is exactly the point. In my everyday job, I program Java 8, maintain different versions, and administer many things. It is possible, but it is effort; it is cost. I was just hoping that when we are not in a corporate world we can be more flexible.

Dear Dan,

you are right, and thank you for the comment. I was a bit pissed when I wrote that sentence. I'm sorry, I fixed it.

Everyone understands that an alpha is a proposal.

I suspected that, but I did not find the document in the project about the release strategy. I did not look for it too long, that is true. Alphas are usually not final releases, but APIs are usually fixed when it is not a SNAPSHOT anymore. You have the right to make incompatible changes in major releases. I am on the sideline so whatever I say is a suggestion from the user side of the API.

We're all trying to create great software

and you do.

@verhas
Copy link
Author

verhas commented Jun 19, 2023

BTW: the version number is outdated in the README.adoc
Jamal is precisely to solve issues like that.

If you say you would consider it, I would be happy to create a pull request renaming README.adoc to README.adoc.jam with the proper macros that fetch the latest version during the build (or when you edit with the plugin we just discussed) and adding Jamal to the build (well, I am not experienced with Gradle, but I will give it a try).

@mojavelinux
Copy link
Member

Thanks @verhas. I appreciate your candid reply.

@mojavelinux
Copy link
Member

Jamal is precisely to solve issues like that.

Cool! Sounds like it could be a good fit.

@robertpanzer
Copy link
Member

robertpanzer commented Jun 25, 2023

Started a thread on Zulip to discuss the path forward.
https://asciidoctor.zulipchat.com/#narrow/stream/279656-dev-.F0.9F.9B.A0.EF.B8.8F/topic/Breaking.20changes.20in.20AsciidoctorJ.203.2E0.2E0/near/369414330

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

No branches or pull requests

4 participants