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

Major update additional changes #63

Open
JanWesterkamp-iJUG opened this issue Sep 22, 2022 · 12 comments
Open

Major update additional changes #63

JanWesterkamp-iJUG opened this issue Sep 22, 2022 · 12 comments

Comments

@JanWesterkamp-iJUG
Copy link
Contributor

JanWesterkamp-iJUG commented Sep 22, 2022

I would like to add some additional updates to the parent besides the Core Profile 10 update (#62).

Most of them should have limited effects on consuming projects, but could have breaking changes to them too - so a good chance to do them now, when it is ensured that all component specs do that update for MP 6.0.

  • dependency version updates (i.e. asciidoctor version)
  • test dependency version updates (i.e. switch JUnit default to 5.x)
  • plugin dependency version updates (i.e. asciidoctor-maven-plugin)
  • adding Asciidoctor Diagram support
  • change spec default source location (i.e. ./src/doc/asciidoc instead of /src/main/asciidoc ?)
  • make consistent use of .adoc (instead of .asciidoc or mixed use) for Asciidoc source files
  • refactoring of maven version definitions to align maven conventions (.version postfix instead of version. prefix)
  • using ISO date format instead of local US date format (2022-09-22 instead of September 22, 2022)

As at least some of this might generate discussions, I can create sub-issues for things we can not get consensus soon, while create separate PRs for the others.

@radcortez and @Emily-Jiang, what do you think about it?

@radcortez
Copy link
Contributor

Feel free to submit a PR with your changes. A few other comments:

  • change spec default source location (i.e. ./src/doc/asciidoc instead of /src/main/asciidoc ?)

Changing this will just break everything. It will require moving the asciidoc files of every MP project to the new folder.

  • make consistent use of .adoc (instead of .asciidoc or mixed use) for Asciidoc source files

Again, this will break downstream projects because the files need to be renamed.

  • refactoring of maven version definitions to align maven conventions (.version postfix instead of version. prefix)

Honestly, I believe it works better this way for auto-completion purposes.

  • using ISO date format instead of local US date format (2022-09-22 instead of September 22, 2022)

This was the format requested to be used when we were putting the release process into place. I have no idea if it is ok to change it or it is an EF requirement to use that format.

@JanWesterkamp-iJUG
Copy link
Contributor Author

Feel free to submit a PR with your changes. A few other comments:

  • change spec default source location (i.e. ./src/doc/asciidoc instead of /src/main/asciidoc ?)

Changing this will just break everything. It will require moving the asciidoc files of every MP project to the new folder.

  • make consistent use of .adoc (instead of .asciidoc or mixed use) for Asciidoc source files

Again, this will break downstream projects because the files need to be renamed.

  • refactoring of maven version definitions to align maven conventions (.version postfix instead of version. prefix)

Honestly, I believe it works better this way for auto-completion purposes.

  • using ISO date format instead of local US date format (2022-09-22 instead of September 22, 2022)

This was the format requested to be used when we were putting the release process into place. I have no idea if it is ok to change it or it is an EF requirement to use that format.

Thanks, @radcortez for your reply!
I will start PRs for the consensus parts, but some thoughts from my side to the other points (and I added/differentiated some too):

Version maven property names do not follow the maven conventions currently (using a dot as separator and a version postfix) - instead we have/had mixed mode use of it (i.e. project.version and java.version recently). Your microprofile-parent setup is realy a great job, but it requires advanced Maven knowledge because of it's complexity ;-) I.e. when deviating from maven conventions, this results in confusion, because people in the component spec projects following the conventions need to do more than expected (instead of simply overwriting the exisiting property they had to define a new property - or accidentially do so - and than had to overwrite the dependency itself with the new property). I strongly suggest to follow these exisiting conventions to make use intuitive and less complex as much as possible.
The current use is not consistent in all microprofile projects (regarding prefix/postfix ans separator use), but I think this major release of the parent is a chance to clean this up - and this should not stop in the parent, of course.
By the way, when I am looking for a specific property for i.e. modification, I would not like not the start writing version and getting a bunch of results - I am prefering starting to typing the project name first and then use the auto-completion from a small set of possible results - but I might miss somthing here?
Most of the use of these properties should be internally to microprofile-parent, but I can check for use/overwrites in component specs.

Regarding the ISO date format, I can check to make sure there is no rule against it, but from my current knowledge, there is no one. I expect this current US local use to be historically derived from the founding loccation of the project (US West Coast) or the Eclipse Foundation Inc. location in Delaware (under State of New York law) legal location - the last one has changed to Eclipse Foundation AISBL in Brussels meanwhile and I would not like ot simply change it to a European locale for that reason. I will ask EMO to make sure on this topic.

For all the (other) breaking changes: I can offer to write a section in the README.adoc (or include on there from a different location) for the migration steps from 2.6 to 3.0, in cluding tips to restore the previous behavior, if there is no time or reason to adopt in the component specs. I also could offer help with PRs on them to adopt too.

I think this major release gives the unique opportunity to do breaking changes, because it will have some already - doing this later causes extra overhead for all consumers and might be accepted only, if Jakarta EE 11 is released (+1a minimum from now) ;-)

I also think the parent should give current defaults, but should make it simple to adopt or overwrite to restore former defaults (i.e. with documentation), when possible.

An example for this could be supporting the latest major versions of test frameworks (i.e. JUnit 5 and TestNG 7) only, but document how to migrate later (therefore I added separate topics for the versions). component specs can decide to adopt now or later with these breaking changes then, but the parent can keep semver rules easily.
This idea cames from the MP Telemetry Tracing project, that wants to make use of JUnit 5, and inverts the current approach to define the new major release use there instead of dropping the old one from the parent and restore them on the current using specs, if necessary until they have migrated to it - Aquillian 1.7.0alpha12 (another update) should support them all.

@Emily-Jiang
Copy link
Member

Feel free to submit a PR with your changes. A few other comments:

  • change spec default source location (i.e. ./src/doc/asciidoc instead of /src/main/asciidoc ?)

Changing this will just break everything. It will require moving the asciidoc files of every MP project to the new folder.

  • make consistent use of .adoc (instead of .asciidoc or mixed use) for Asciidoc source files

Again, this will break downstream projects because the files need to be renamed.

  • refactoring of maven version definitions to align maven conventions (.version postfix instead of version. prefix)

Honestly, I believe it works better this way for auto-completion purposes.

  • using ISO date format instead of local US date format (2022-09-22 instead of September 22, 2022)

This was the format requested to be used when we were putting the release process into place. I have no idea if it is ok to change it or it is an EF requirement to use that format.

Thanks, @radcortez for your reply! I will start PRs for the consensus parts, but some thoughts from my side to the other points (and I added/differentiated some too):

Version maven property names do not follow the maven conventions currently (using a dot as separator and a version postfix) - instead we have/had mixed mode use of it (i.e. project.version and java.version recently). Your microprofile-parent setup is realy a great job, but it requires advanced Maven knowledge because of it's complexity ;-) I.e. when deviating from maven conventions, this results in confusion, because people in the component spec projects following the conventions need to do more than expected (instead of simply overwriting the exisiting property they had to define a new property - or accidentially do so - and than had to overwrite the dependency itself with the new property). I strongly suggest to follow these exisiting conventions to make use intuitive and less complex as much as possible. The current use is not consistent in all microprofile projects (regarding prefix/postfix ans separator use), but I think this major release of the parent is a chance to clean this up - and this should not stop in the parent, of course. By the way, when I am looking for a specific property for i.e. modification, I would not like not the start writing version and getting a bunch of results - I am prefering starting to typing the project name first and then use the auto-completion from a small set of possible results - but I might miss somthing here? Most of the use of these properties should be internally to microprofile-parent, but I can check for use/overwrites in component specs.

Regarding the ISO date format, I can check to make sure there is no rule against it, but from my current knowledge, there is no one. I expect this current US local use to be historically derived from the founding loccation of the project (US West Coast) or the Eclipse Foundation Inc. location in Delaware (under State of New York law) legal location - the last one has changed to Eclipse Foundation AISBL in Brussels meanwhile and I would not like ot simply change it to a European locale for that reason. I will ask EMO to make sure on this topic.

For all the (other) breaking changes: I can offer to write a section in the README.adoc (or include on there from a different location) for the migration steps from 2.6 to 3.0, in cluding tips to restore the previous behavior, if there is no time or reason to adopt in the component specs. I also could offer help with PRs on them to adopt too.

I think this major release gives the unique opportunity to do breaking changes, because it will have some already - doing this later causes extra overhead for all consumers and might be accepted only, if Jakarta EE 11 is released (+1a minimum from now) ;-)

I also think the parent should give current defaults, but should make it simple to adopt or overwrite to restore former defaults (i.e. with documentation), when possible.

An example for this could be supporting the latest major versions of test frameworks (i.e. JUnit 5 and TestNG 7) only, but document how to migrate later (therefore I added separate topics for the versions). component specs can decide to adopt now or later with these breaking changes then, but the parent can keep semver rules easily. This idea cames from the MP Telemetry Tracing project, that wants to make use of JUnit 5, and inverts the current approach to define the new major release use there instead of dropping the old one from the parent and restore them on the current using specs, if necessary until they have migrated to it - Aquillian 1.7.0alpha12 (another update) should support them all.

@JanWesterkamp-iJUG All of the above proposal needs further discussion. I am afraid it might be too late for MP 6. You can open a discussion on how to organise the files in the dicussion thread via emails.
As for the following:

  • dependency version updates (i.e. asciidoctor version)
  • test dependency version updates (i.e. switch JUnit default to 5.x)
  • plugin dependency version updates (i.e. asciidoctor-maven-plugin)
    go ahead to make a PR with your proposed change.

@JanWesterkamp-iJUG
Copy link
Contributor Author

@Emily-Jiang not all of it will be breaking changes and most of it could be compensated easily - but answering this now, after a 3.0 release was done last week, while ignoring my conerns and offerings before is not the best way managing this project, to be honest.

Now everything that breaks would require another major release (4.0.0), when not addressing bugs directly!

I will create a PR for adding Asciidoctor Diagram support first now (basially adding a feature and updating some required dependencies for this), then we can discuss the rest tomorrow.

@JanWesterkamp-iJUG
Copy link
Contributor Author

@radcortez and @Emily-Jiang, the PR for the AsciiDoctor Diagram support is created:
Asciidoctor Diagram support

Can you please review it?

@Emily-Jiang
Copy link
Member

@Emily-Jiang not all of it will be breaking changes and most of it could be compensated easily - but answering this now, after a 3.0 release was done last week, while ignoring my conerns and offerings before is not the best way managing this project, to be honest.

Now everything that breaks would require another major release (4.0.0), when not addressing bugs directly!

I will create a PR for adding Asciidoctor Diagram support first now (basially adding a feature and updating some required dependencies for this), then we can discuss the rest tomorrow.

@JanWesterkamp-iJUG We need to do incremental changes. Releasing 3.0 is not the end of development. We can release 3.x whenever there is a need. The reason to release 3.0 is that Metrics needs to rebase it sooner. When no updates are available, we can release a minor change. We should not pack too many things in one release.

@radcortez
Copy link
Contributor

Instead of breaking some of the stuff, maybe we can just change it to support the current structure and the new one (if the community wants to go that way) in the same pom.

It should be possible to add additional plugin executions to search for different folder structures. You can, for sure, define new properties with a new convention and keep the old ones, etc.

@JanWesterkamp-iJUG
Copy link
Contributor Author

Instead of breaking some of the stuff, maybe we can just change it to support the current structure and the new one (if the community wants to go that way) in the same pom.

It should be possible to add additional plugin executions to search for different folder structures. You can, for sure, define new properties with a new convention and keep the old ones, etc.

@radcortez: In fact, with the PR for AsciiDoctor Diagram support I did a renaming of two existing maven properties (asciidoctor.maven.plugin.version and asciidoctorj.pdf.version) to align them to the official AsciiDoctor documentation and examples, which would be a breaking change if somebody overwrites them already - I can check if that has happened. Then we can discuss to change it back to the old existing names until complete refactoring of the names will be done.

Regarding the AsciiDoc file location and postfixes:

Currently there is no location defined explicitly - the plugin scanes for three directories automatically:

  • src/docs/asciidoc
  • src/asciidoc
  • src/main/asciidoc (current used location)

Personally, I am prefering src/doc/asciidoc (singular notation) instead, but that is not a default configuration in the plugin and Maven has not defined a folder in the Standard Directory Layout for now. I think we could address this topic there first and fix it when defined - until then we can rely on the automatic scaning of the maven plugin (without breaking things).

Regarding the ending, I would like to do the change to the .adoc postfix, so we have consistency there, but this would be a breaking change (that can be soved easily of course).

In principal, using multiple execution definitions would be a good idea, but unfortunately the build creates errors because of missing files and not define the root document at all creates warnings for duplications/overwrites when there are multiple linked documents.

@JanWesterkamp-iJUG
Copy link
Contributor Author

@Emily-Jiang here is the first (non breaking) part of the test dependencies update as PR:
#67

@JanWesterkamp-iJUG
Copy link
Contributor Author

* plugin dependency version updates (i.e. asciidoctor-maven-plugin)
  go ahead to make a PR with your proposed change.

I started addessing this point and created a PR for the in Maven 3.8.6 included plugins. Please review.

@JanWesterkamp-iJUG
Copy link
Contributor Author

@radcortez: In fact, with the PR for AsciiDoctor Diagram support I did a renaming of two existing maven properties (asciidoctor.maven.plugin.version and asciidoctorj.pdf.version) to align them to the official AsciiDoctor documentation and examples, which would be a breaking change if somebody overwrites them already - I can check if that has happened. Then we can discuss to change it back to the old existing names until complete refactoring of the names will be done.

@radcortez I verified that the exisitng version property names, that I renamed, are not reused in MicroProfile projects (at least in the main/master branch) - so it should be safe to rename them, because of microprofile-parent internal use only.

The only potential breaking change of the current PR #66 is the major update of the PDF generator and the deviating use of the code-highlighter. I will try to further examine the impact on this, but all specs I have seen so far overwriting the setting in the doc themself (to coderay like this ":source-highlighter: coderay"). The original support of this setting was removed from the maven plugin (https://docs.asciidoctor.org/maven-tools/latest/plugin/v2-migration-guide/#removal-of-sourcehighlighter), so the existing support is broken anyway (!)

@radcortez
Copy link
Contributor

If the property names are not in use by downstream projects, feel free to propose the change.

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

3 participants