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

TOMEE-3846 improve main comparison page and fix per version comparison pages #37

Merged
merged 5 commits into from May 3, 2022

Conversation

sultan
Copy link
Contributor

@sultan sultan commented Apr 2, 2022

work in progress, draft so everyone in the mailing list can see the results

@sultan
Copy link
Contributor Author

sultan commented Apr 2, 2022

image

Flavors only (version A) :

image

Versions only (version B) :

image

Both (version C) :

image

implementations :

image

@cesarhernandezgt
Copy link
Contributor

Hi @sultan
thank you much for the draft PR, I like a lot your proposals, I have the following feedback on the draft pr:

From the first image you published, I like the structure of the following but I would reduce the number of columns and change the order:

TomEE Branch || Java SE || JakartaEE || MicroProfile
or
TomEE Branch || Java SE || JakartaEE || MicroProfile || Based on Tomcat

Why?

  • The Order (left to right) focus first the attention in the "TomEE branching", then in the second most common question "What SE version should I use", and then finish with the Jakarta and Microprofile Macro platforms.
  • By removing "TomEE Release" column, this table won't need manual mantainance for each TomEE minor release.
  • The number of specificaiton is a long list than doesn't seems to scale find in a horizontal approach, that's why in my humble opinion the specific versions of each specification can be linked into a separated table.

"Flavors only (version A)"

+1, I like this table since cover a perspective the table above doesn't.

There is a third table that can contain de specific version of each spec peer branch and flavor. Notice that my draft is based on the current Flavor table (https://tomee.apache.org/comparison.html).
image

@sultan
Copy link
Contributor Author

sultan commented Apr 6, 2022

Thank you for your feedback, i removed 'release column' to ease maintenance.

i also believe we can't put all specs versions in columns as it takes too much width. some remarks :

i find that table without specs to be a bit useless because tomee follows jakarta ee numbering since v7 :
image
so i thought some specs could be usefull. i selected them based on what can help my students configure TomEE and Eclipse :
image

the table you are suggesting is actually my table "C", sorry if i wasnt clear :
image

@cesarhernandezgt
Copy link
Contributor

image
+1, I like a lot this table. It's the kind of table fit nicely in a presentation slide and reply most common questions we get about compatibility.

the table you are suggesting is actually my table "C", sorry if i wasnt clear :

No worries. yes! table C is the ultimate Spec version, TomEE and flavor matrix of the matrix :).

@sultan sultan changed the title TOMEE- draft improve main comparison page and fix per version comparison pages TOMEE-3846 draft improve main comparison page and fix per version comparison pages Apr 7, 2022
@sultan
Copy link
Contributor Author

sultan commented Apr 7, 2022

ok so i wrapped everything, now i have only one page with everything, there is still room for change if need be.

edit : added anchor ids

my problem now is build is broken on windows again (javadoc not generated, slash errors ?)

i need either, help to make build works on windows for javadoc,
or, help to test and replace :

java
new Source("https://github.com/eclipse/microprofile-bom.git", "master", "microprofile-5.0").related(microProfile5).javadoc("^org.eclipse.microprofile."),
new Source("https://github.com/eclipse/microprofile-bom.git", "master", "microprofile-2.0").related(microProfile2).javadoc("^org.eclipse.microprofile.
"),

by:

java
new Source("https://github.com/eclipse/microprofile.git", "5.0", "microprofile-5.0").related(microProfile5).javadoc("^org.eclipse.microprofile."),
new Source("https://github.com/eclipse/microprofile.git", "2.0", "microprofile-2.0").related(microProfile2).javadoc("^org.eclipse.microprofile.
"),

i can also try on a mac later

@sultan
Copy link
Contributor Author

sultan commented Apr 8, 2022

Current draft is :
image

@rzo1
Copy link
Contributor

rzo1 commented Apr 8, 2022

ok so i wrapped everything, now i have only one page with everything, there is still room for change if need be.

edit : added anchor ids

my problem now is build is broken on windows again (javadoc not generated, slash errors ?)

i need either, help to make build works on windows for javadoc, or, help to test and replace :

java new Source("https://github.com/eclipse/microprofile-bom.git", "master", "microprofile-5.0").related(microProfile5).javadoc("^org.eclipse.microprofile."), new Source("https://github.com/eclipse/microprofile-bom.git", "master", "microprofile-2.0").related(microProfile2).javadoc("^org.eclipse.microprofile."),

by:

java new Source("https://github.com/eclipse/microprofile.git", "5.0", "microprofile-5.0").related(microProfile5).javadoc("^org.eclipse.microprofile."), new Source("https://github.com/eclipse/microprofile.git", "2.0", "microprofile-2.0").related(microProfile2).javadoc("^org.eclipse.microprofile."),

i can also try on a mac later

Is this still a problem? Would need to start a Windows VM to check / debug ;)

@sultan
Copy link
Contributor Author

sultan commented Apr 10, 2022

i was able to fix the windows build, ill upload it later. not a problem anymore,
we can continue on this PR. open to comments, i am waiting for tomee3888 to be merged before this can be ready

@sultan sultan marked this pull request as ready for review April 10, 2022 18:18
@sultan
Copy link
Contributor Author

sultan commented Apr 10, 2022

@rzo1 not squashed but ready for review, thanks in advance

@sultan
Copy link
Contributor Author

sultan commented Apr 10, 2022

i finally learned how to squash, the compare button says its ok

@sultan sultan changed the title TOMEE-3846 draft improve main comparison page and fix per version comparison pages TOMEE-3846 improve main comparison page and fix per version comparison pages Apr 10, 2022
Copy link
Contributor

@rzo1 rzo1 left a comment

Choose a reason for hiding this comment

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

I did a local build and looked at the comparision page. See my comments / notes / questions below ;)

Main differences between versions

  • Any reason for "(!)" in the header for "Based on Tomcat (!)"? Emphasizing that it isn't TomEE? Just identified, that this is a footnote. Maybe we can choose an other symbol for it?
  • Any reason for the "..." in the table header?
  • Whitespace issue in "resprctive Apache tomEE version is_:"

Detailed list of Jakarta EE and MicroProfile specifications

Might be dump questions but:

  • What is the explanation for some items being "bolt"? We might need to explain it.
  • TomEE 9.x is Java SE Runtime 11
  • Don't know if we should include TomEE 10.x at all - as we do not have any tech preview for it and didn't work on it (yet). But if we mention this cleary, that this is a "preview", I wouldn't care.
  • Maybe we can link to the vendor sites for "MyFaces", "Mojarra", "OpenJPA" ?
  • Why is Mojarra and EclipseLink marked in "bolt" for Plume?

Implementations of Jakarta EE and MicroProfile features in TomEE

  • Maybe we can place the (...) comments into footnotes and link them with 1,2,3 etc. - this would make it easier to read and remove duplicate content.
  • Why are certain implementations written in "bolt"? I do not see the rational atm - maybe we need to explain or change.

@sultan
Copy link
Contributor Author

sultan commented Apr 11, 2022

thanks ! i will make changes to reflect your suggestions,

another question in head :
we do provide javadoc because of lack from eclipse/oracle and make traffic to TomEE website (its great) 👍
do we both need Jakarta EE 9.0 and 9.1 javadoc on site ? we totally can but maybe 9.1 only is enough ? what do you think ?

bold for specs was because there a majors specs like jakarta ee platform / webprofile / microprofile
but i mus think on how to make them shine better

bold for implementation was for implementations differences between flavors ( its true i may better say it so on the page )

@sultan sultan marked this pull request as draft April 11, 2022 19:23
@cesarhernandezgt
Copy link
Contributor

Thank you @sultan for the PR updates and @rzo1 for the review.
So awesome to see that the Windows issue was fixed!

do we both need Jakarta EE 9.0 and 9.1 javadoc on site ? we totally can but maybe 9.1 only is enough? what do you think ?

I would vote +1 for only 9.1. But my opinion is that this work/proposal can be part of a new ticket to limit the scope of the current ticket to only comparison page content.

I won't duplicate Richard's feedback, but the only table I don't fully understand is why the first table [1] has some specifications when the full list is already provided with major detail in the table: "Detailed list of Jakarta EE and MicroProfile specifications".

You previously mentioned:

so I thought some specs could be usefull. i selected them based on what can help my students configure TomEE and Eclipse :

But I still don't get the problem this part of the table is solving. I used Eclipse IDE and configuration steps for TomEE in Eclipse are documented here: https://tomee.apache.org/latest/docs/tomee-and-eclipse.html. If your students are creating projects from scratch, maybe a maven archetype could be better to customize their project setup in terms of dependencies and version numbers.

[1]
image

@sultan
Copy link
Contributor Author

sultan commented Apr 12, 2022

i wont say too much on why the schools i teach in forbid the use of maven/gradle at the stage my learners are ... there are solutions for me without this :-) (or add that to my course instead of TomEE docs)

anyway your comments shed more light on how i could make it more readable, so we might need less tables in the end.

javadoc main page and comparison page are tied together because of TomEE 9 update to MicroProfile 5, it made sense to me to have them updated in same PR.

i'll send something as soon as can, thanks for your feedbacks.

@sultan
Copy link
Contributor Author

sultan commented Apr 12, 2022

To answer your question, the problem that could be solved by a short spec table was the mess eclipse is for my students

image

but again i have other means to counter the problem, (if not by TomEE doc) <3

@sultan
Copy link
Contributor Author

sultan commented Apr 12, 2022

the only problem i wont be able to solve, is that, if i shorten the first table, then i lose some info that are not mirrored in the detailed table.
e.g. TomEE 7.x are not in the detailed table:

image

image

and now the specs for EE 7 servlets etc are gone ...

@dblevins
Copy link
Contributor

Hey All,

I posted some thoughts to the dev list. If people can read and respond there, that'd be great.

@sultan
Copy link
Contributor Author

sultan commented Apr 17, 2022

Following @cesarhernandezgt feedbacks, this PR nows depends on 2 others:

those per-version comparison pages (v8,v9) are ready for review

@sultan
Copy link
Contributor Author

sultan commented Apr 19, 2022

commits squashed

@sultan
Copy link
Contributor Author

sultan commented Apr 19, 2022

Thank you @sultan for the PR updates and @rzo1 for the review. So awesome to see that the Windows issue was fixed!

do we both need Jakarta EE 9.0 and 9.1 javadoc on site ? we totally can but maybe 9.1 only is enough? what do you think ?

I would vote +1 for only 9.1. But my opinion is that this work/proposal can be part of a new ticket to limit the scope of the current ticket to only comparison page content.

@cesarhernandezgt i made a separated ticket for the javadoc #40

@cesarhernandezgt
Copy link
Contributor

Thank you for the updates @sultan. So far, I haven't been able to pull your changes, but I hope to have time by the end of the week and provide feedback if needed by then.

@sultan
Copy link
Contributor Author

sultan commented Apr 21, 2022

no worries, I just made changes following David's feedback

@sultan sultan marked this pull request as ready for review April 22, 2022 06:44
@sultan
Copy link
Contributor Author

sultan commented Apr 22, 2022

this PR requires #40

@sultan
Copy link
Contributor Author

sultan commented Apr 22, 2022

all pull requests needed (for this one) are merged, this PR can be reviewed/merged

@rzo1
Copy link
Contributor

rzo1 commented Apr 27, 2022

Let''s wait a few more days and if no one objects, we should go a-head with it and iterate / rework it if needed.

@rzo1 rzo1 merged commit 97bb618 into apache:master May 3, 2022
@rzo1
Copy link
Contributor

rzo1 commented May 3, 2022

@sultan Thanks for the contribution! If we find some issues, we can iterate / rework based on this work!

@sultan sultan deleted the fix-comparison-pages branch May 3, 2022 06:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants