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

Road to Asciidoctor 2.0.0 #391

Closed
abelsromero opened this issue Mar 30, 2019 · 13 comments
Closed

Road to Asciidoctor 2.0.0 #391

abelsromero opened this issue Mar 30, 2019 · 13 comments
Milestone

Comments

@abelsromero
Copy link
Member

abelsromero commented Mar 30, 2019

It's been a while but let's roll the 2.0.0 asciidoctor goodies for maven too.
The plan is split in 2 steps

1. Release 1.5.8 version (DONE)

This is more of a farewell and check to make sure we don't miss anything.
This will be aligned with Asciidoctor 1.5.8.1 (+ testing of 1.6.x) and contain some minor fixes.

Due to AsciidoctorJ changes, this won't be compatible with AsciidoctorJ 2.0.x.

To be released today 30th March.

2. Release 2.0.0 version

It will contain the necessary changes to use AsciidoctorJ 2.0.x., thx @robertpanzer for doing most of the work here.

This will introduce non-backward compatible changes (see migration guide https://gist.github.com/abelsromero/263ae7703f4bc5efebbfd16d9e407c28) that have been stalled untill now.
The complete list is here: https://github.com/asciidoctor/asciidoctor-maven-plugin/milestone/14.
Most notably are:

  • Removal of imagesDir and sourceHighlighter options in favour of defining it in the <attributes> section.
  • Replacement of templateDir (now deprecated in Asciidoctor) by templateDirs (List) (#265)
  • Removing support for Java 7

To be released (ideally) tomorrow or day after unless there are objections

asciidoctor-maven-examples will be updated during the next week after release.

What next?
After initial 2.0.0 release a new way for the maven-plugin opens with adoption of semantic versioning independent from the other Asciidoctor modules it relies on (Asciidoctor and AsciidocotorJ).
This gives us the opportunity and responsibility to evolve at our own pace.
For example, features like using other Asciidoctor implementations (e.g. asciidoctor.js) are not that far. These will likely require changes in the configuration that will break backward compatibility. Thanks to the new versioning model we will be able to achieve that in a less intrusive way possible for users.

@rmannibucau
Copy link
Contributor

Hi

Not a blocker but is there any rational to not maintain backward compatibility?
AFAIK attributes can be set from the mojo if not already present and therefore maintain all existing config "as it". This also makes sense for very common ones IMHO to have completion in the editor - attributes dont have it.

@abelsromero
Copy link
Member Author

abelsromero commented Mar 31, 2019

Not a blocker but is there any rational to not maintain backward compatibility?

Ever since some of the ideas finally implemented in 2.0.0 where scheduled for 1.6.0, there has been a plan in the whole Asciidoctor ecosystem to introduce some changes that required breaking compatibility.
We are using using 2.0.0, precisely following semantic versioning, an an opportunity to ensure everyone (ruby, js, Java + plugins) is aware this a breaking point to make things easier. I do admit however, that maven changes are being done with short notice. But we can take some days to discuss if necessary.

AFAIK attributes can be set from the mojo if not already present and therefore maintain all existing config "as it".

Just checked and this is not possible. Maven plexus just maps the XML to the Mojo pojo. You can define a Map property like we do with attributes to be able to inject things dynamically, but this is not possible at Mojo level. Unless there's some way that I am not aware of, for example some dynamic setter like Groovy's setProperty.

We could maintain the same configuration model, but there some current options that in my opinion are clearly wrong:

  • imagesDir sets a default value different from asciidodcotor, this had led to confusions in the past and needs fixing (Remove default value for imagesdir configuration. #296). In this case I am opting for totally removing the option and allowing imagesdir configuration only in the attributes section. This is for 3 reasons:
    1. Fixing the current divergence with asciidoctor.
    2. Removing ambiguity. Now it is possible to set the property in both the configuration element and the attributes section. The second overriding the first. I think this can confuse users and also requires some additional logic in the plugin.
    3. Making a clear distinction between asciidoctor options and attributes. This is a nice thing to ease migration to more complete building solutions, specially now that https://antora.org/ is available and mature. My intention with 2.0.x and ahead is ensuring the maven plugin is as minimal and possible avoiding any "vendor lock" effect.
  • templateDir is now a plural multi-valued property and the singular one is deprecated. We could maintain both but I think this will add confusion. Fix here is pretty simple and I don't think it imposes a heavy pain in plugin users.

About the removal of sourceHighlighter, is for previous reasons 2 and 3.

I do agree that autocompletion is a nice feature, but is just 3 properties, and documentation + the maven-examples repo are available.

Given you feedback I'll start writing the migration guide to the README. This will help me see better the impact.

PS: Note more changes will come in the future. For example gemsPath will need to be restructured when we support multiple implementations other than Ruby. Any non-backward compatible change will be a major version.

@rmannibucau
Copy link
Contributor

About plexus, you can inject the xml and parse it as you want so technically there is no blocker. Also not sure why it wouldnt be mapped in the pojo. In 1.5 you were already able to use attributes so there is no reason to consider it as breaking since it is the case since years.
Also failing for ambiguous cases is fine - in practise it is never ambiguous since you set it only on one side ;)

@abelsromero
Copy link
Member Author

abelsromero commented Mar 31, 2019

About plexus, you can inject the xml and parse it as you want so technically there is no blocker.

Good catch! I can give it a try for 2.0.0 and mark the options as deprecated to be removed at some point to make transition less traumatic. Fields in some IDE (at least IntelliJ) will appear highlighted and not valid though.

@abelsromero
Copy link
Member Author

Here is the full migration guide https://gist.github.com/abelsromero/263ae7703f4bc5efebbfd16d9e407c28.

@rmannibucau
Copy link
Contributor

Here a few comments:

  1. imagesDir compat can be fully kept since you can 1. check there is the attribute -> ensure 2 is not elligible or fail and then only use attribute value, if not 2. check there is an imageDir value and if so use it, if not 3. set images/,
  2. same applies to templateDir I guess (concretely you rarely use multiple folders so having a shortcut for a single one is still a feature of the mojo IMHO),
  3. I don't get at all the src/docs folder, it sounds wrong. Main is for deliverables and Test for harnessing, doc clearly belongs to main and it is the common usage to put it - even without that plugin - so i'd like to not change that default for something breaking maven habits if possible

Long story short I don't think that the plugin should stick to the asciidoctorj API only, this is not how a MOJO is useful but it should enable the config to be as explicit and user friendly as possible so these breakage are against that IMHO and not justified technically as well - the code to not break is very localized and manageable in time. To illustrate that it is trivial to use gmaven-plus plugin to render yourself your document programmatically in an aligned way on asciidoctorj so the plugin value is to encourage good practises IMHO. The v2 proposal is against that IMHO.

Any hope this decision get revisited before the release?

@abelsromero
Copy link
Member Author

Keeping the old fields for 2.0.0 is still on the table, at least for a RC (which is my current goal for today, more info at the end). But eventually I still think removing them is for the best and a major version change is a good opportunity.
Your example on imagesDir already highlights how keeping some options creates additional complexity. Following on templateDir, if both fields are available we need to consider what to do when both are set, merge? or override? or abort on error? Not to mention if we need to parse the XML directly.

You are right these is not extremely hard to maintain and are not mandatory, but over the past years the plugin has introduced several quirks, some because things where not well though at the time, like the imagesDir default value. Others just because how configurations options/attributes relate to each other.
And all of them add some amount already, so I really want to take the opportunity to simplify things, kind of a blank slate to ensure the maintainability of the plugin in the long run.

Any hope this decision get revisited before the release?

Right now I am focusing on release a RC with non-braking changes, that is:

  • Remove support for Java 7
  • Ensure Java 10 and 11 work
  • Add templateDirs only
    All fields will be kept as normal properties for RC

@mojavelinux
Copy link
Member

I don't get at all the src/docs folder, it sounds wrong.

If you don't agree with the change to src/docs/asciidoc, please comment on the issue in which the change is being discussed. That's #254. I believe the argument I made is a strong one and even a Maven developer supported it (after some back and forth). I realize the change could be rather disruptive, which would be the strongest case for not doing it. But I feel strongly it should have been src/docs/asciidoc from the beginning.

@wimdeblauwe
Copy link

Is there any idea when the currently published 2.0.0-RC.1 will become a final 2.0.0 ?

@abelsromero
Copy link
Member Author

May sound as excuse, but I am on it now. I cleared my schedule 2 weeks ago and I am working on compatibilities issues with gem-maven-plugin. As soon as these are fixed I'll do the changes and release the asciidoctor-maven-plugin.
The 2.0.0 is perfectly safe to use.

@abelsromero abelsromero added this to the 2.0.0 milestone May 31, 2020
@abelsromero
Copy link
Member Author

Everything is ready, relase will be done during the weekend if nothing goes wrong.

@abelsromero
Copy link
Member Author

@mojavelinux
Copy link
Member

@abelsromero Incredible work on the 2.0.0 release! Congratulations! 🎉 🍻

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