ICU-22324 Add docs for Maven usage #2655

echeran · 2023-10-03T23:35:05Z

I did a manual run of the "Deploy User Guide" workflow in my personal fork so that you can preview the effects of the changes.

Root of deployed site
- ICU4J Readme: https://echeran.github.io/icu/userguide/icu4j/
- Maven Setup for Java: https://echeran.github.io/icu/devsetup/java/maven.html

Checklist

Required: Issue filed: https://unicode-org.atlassian.net/browse/ICU-22324
Required: The PR title must be prefixed with a JIRA Issue number.
Required: The PR description must include the link to the Jira Issue, for example by completing the URL in the first checklist item
Required: Each commit message must be prefixed with a JIRA Issue number.
Issue accepted (done by Technical Committee after discussion)
Tests included, if applicable
API docs and/or User Guide docs changed or added, if applicable

jira-pull-request-webhook · 2023-10-04T17:30:31Z

Notice: the branch changed across the force-push!

docs/devsetup/java/maven.md is different

View Diff Across Force-Push

~ Your Friendly Jira-GitHub PR Checker Bot

markusicu

lgtm tnx!

I did not read every word...

jira-pull-request-webhook · 2023-10-04T21:17:51Z

Notice: the branch changed across the force-push!

docs/devsetup/java/maven.md is different

View Diff Across Force-Push

~ Your Friendly Jira-GitHub PR Checker Bot

mihnita · 2023-10-05T01:21:22Z

docs/userguide/icu4j/index.md

+        <dependency>
+            <groupId>com.ibm.icu</groupId>
+            <artifactId>icu4j</artifactId>
+            <version>68.1</version>


Nitpick: newer version? (74.1, or at least 73.2 ?)

If 68.1 is already used in other places then either change all, or keep 68.1 here and do an update later (your call)

Ack. I'm okay with an old version. We shouldn't feel the need to update this during a new version release's BRS tasks. If it's not old now, it will become so after the next version.

mihnita · 2023-10-05T01:40:32Z

docs/userguide/icu4j/index.md

+Any resource files (non-source files) will be located at `src/main/resources`.
+
+
+| Sub-component Path                    | Sub-component Name | Build Dependencies        | Public API Packages | Description   |


I wonder if it is worth listing this.
Or at least the dependency, and component names columns.

These are things that can be easily obtained with maven:

mvn dependency:tree

I included that to be consistent with the Ant-based docs we have for ICU 73 and earlier. The same argument could have been made there ("just look at the build.xml file to find component deps").

Personally, I think documentation about a codebase is something that I find useful and would like to see more of, even though it is rare. But I do believe it to be a different type of doc than inline source comments, API docs, design/architecture docs, and tutorials.

Yes, but documentation about code base also gets outdated very fast :-)

For the Ant dependencies you had to grep, then "massage" things yourself.
Maven has a standard way to do it.

What about keep the table, but remove the "Dependencies" column, and say "for an up to date dependency report run mvn dependency:tree"?
The result is more readable than this table (for example you can see if something is a test dependency or not)

mihnita · 2023-10-05T01:45:08Z

docs/userguide/icu4j/index.md

+    </tr>
+    <tr>
+        <td><code>main/shared</code></td>
+        <td>(Obsolete?) Files shared by ICU4J sub-components under the <code>main</code> directory including: <ul> <li>ICU4J runtime data archive (icudata.jar).</li> <li>ICU4J unit test data archive (testdata.jar).</li> <li>Shared Ant build script and configuration files.</li> <li>License files.</li></ul> </td>


+10
Obsolete, can remove the whole line.

mihnita · 2023-10-05T01:48:00Z

docs/userguide/icu4j/index.md

+    </tr>
+</table>
+
+## The Structure and Contents of ICU4J - ICU 73 and earlier


Idea: split in a different file, to reduce the clutter?
And with only a link from here?

Markus wanted to keep the Ant instructions for people who might still be using a version of ICU4J that is a few versions old. There will be a non-trivial number of people.

Yes, I didn't say remove.
Only split into a different file with a link.

Let's keep it here for now, at least for expediency.

mihnita · 2023-10-05T01:49:08Z

docs/userguide/icu4j/index.md

+Once the JDK and Maven are installed, run the desired Maven target. For example:
+
+~~~
+~/icu/icu4j$ mvn test


mvn verify
?
Let's not encourage people to skip the integration tests :-)

mihnita

Added a few comments.

mihnita

Updated.
And sorry, I realized I missed maven.md. Will do.

mihnita · 2023-10-05T17:41:28Z

docs/devsetup/java/maven.md

+---
+
+
+Maven is a pretty standard build tool in the Java ecosystem with a very well-defined


Nitpick: remove "pretty" :-) After 20+ years it is standard :-)

mihnita · 2023-10-05T17:45:53Z

docs/devsetup/java/maven.md

+## Installation
+
+Install Maven from your OS package manager.
+The minimum version is 3.2.5, as configured in the `<ICU>/icu4j/pom.xml` root POM file's maven-enforcer-plugin.


Do we want this level of detail?
This is the kind of stuff that gets outdated sooner or later.

Link to https://books.sonatype.com/mvnex-book/reference/installation.html ?

In general I would make this doc smaller and link to the relevant sections of "Maven by Example" or "Maven: The Complete Reference" as needed.

docs/devsetup/java/maven.md

mihnita

Thank you.

Added a few comments, nitpicks.
If you like (some) of them, don't be reluctant to implement them.
I will re-approve in a minute.

mihnita · 2023-10-05T19:33:52Z

docs/devsetup/java/maven.md

+
+## Usage at the command line
+
+Maven divides its concept of a build into a "lifecycle" of a linear sequence of steps, called "phases".


Link?
https://books.sonatype.com/mvnref-book/reference/lifecycle-sect-structure.html#lifecycle-sect-default

mihnita · 2023-10-05T19:35:00Z

docs/devsetup/java/maven.md

+
+where `<projectlist>` is a comma-separated list of names of the subfolders which contain the submodule configuration pom.xml files.
+
+If you want to run only a specific test(s), use the `-Dtest="<test>"` option, where `<test>` can be a test name, a class name / package prefix, or a comma-separate list of them.


Maybe drop this paragraph, since you have a whole "Run a single test" section?

mihnita · 2023-10-05T19:35:31Z

docs/devsetup/java/maven.md

+mvn integration-test -am -pl main/core
+```
+
+#### Run a single test


Link:
https://maven.apache.org/surefire/maven-surefire-plugin/examples/single-test.html

mihnita · 2023-10-05T19:37:36Z

docs/devsetup/java/maven.md

+## More info on Maven
+
+To learn more about the details of Maven not covered above,
+start by reading the [*Maven by Example* book](https://books.sonatype.com/mvnex-book/reference/index.html),


Nitpick / personal pref: "book" outside the link.

See unicode-org#2655

jira-pull-request-webhook · 2023-10-05T19:51:51Z

Hooray! The files in the branch are the same across the force-push. 😃

~ Your Friendly Jira-GitHub PR Checker Bot

echeran requested a review from mihnita October 3, 2023 23:35

echeran assigned markusicu Oct 3, 2023

echeran requested a review from yumaoka October 4, 2023 15:34

echeran force-pushed the maven-docs branch from 9d1f450 to 71bb063 Compare October 4, 2023 17:30

markusicu previously approved these changes Oct 4, 2023

View reviewed changes

echeran dismissed markusicu’s stale review via 7a05943 October 4, 2023 21:17

echeran force-pushed the maven-docs branch from 71bb063 to 7a05943 Compare October 4, 2023 21:17

markusicu previously approved these changes Oct 4, 2023

View reviewed changes

mihnita reviewed Oct 5, 2023

View reviewed changes

echeran dismissed markusicu’s stale review via 49d69f5 October 5, 2023 04:23

echeran requested review from mihnita and markusicu October 5, 2023 04:23

mihnita reviewed Oct 5, 2023

View reviewed changes

docs/devsetup/java/maven.md Show resolved Hide resolved

mihnita reviewed Oct 5, 2023

View reviewed changes

docs/devsetup/java/maven.md Show resolved Hide resolved

mihnita reviewed Oct 5, 2023

View reviewed changes

docs/devsetup/java/maven.md Show resolved Hide resolved

mihnita approved these changes Oct 5, 2023

View reviewed changes

ICU-22324 Add docs for Maven usage

44f3abf

See unicode-org#2655

echeran force-pushed the maven-docs branch from 2a2f797 to 44f3abf Compare October 5, 2023 19:51

echeran merged commit 2e9d128 into unicode-org:main Oct 5, 2023
99 checks passed

echeran deleted the maven-docs branch October 5, 2023 20:43

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ICU-22324 Add docs for Maven usage #2655

ICU-22324 Add docs for Maven usage #2655

echeran commented Oct 3, 2023 •

edited

jira-pull-request-webhook bot commented Oct 4, 2023

markusicu left a comment

jira-pull-request-webhook bot commented Oct 4, 2023

mihnita Oct 5, 2023 •

edited

echeran Oct 5, 2023

mihnita Oct 5, 2023 •

edited

echeran Oct 5, 2023

mihnita Oct 5, 2023

mihnita Oct 5, 2023

echeran Oct 5, 2023

mihnita Oct 5, 2023

echeran Oct 5, 2023

mihnita Oct 5, 2023

markusicu Oct 5, 2023

mihnita Oct 5, 2023

echeran Oct 5, 2023

mihnita left a comment

mihnita left a comment

mihnita Oct 5, 2023

echeran Oct 5, 2023

mihnita Oct 5, 2023

echeran Oct 5, 2023

mihnita left a comment

mihnita Oct 5, 2023

mihnita Oct 5, 2023

mihnita Oct 5, 2023

mihnita Oct 5, 2023

jira-pull-request-webhook bot commented Oct 5, 2023

		Any resource files (non-source files) will be located at `src/main/resources`.


		\| Sub-component Path \| Sub-component Name \| Build Dependencies \| Public API Packages \| Description \|

		---


		Maven is a pretty standard build tool in the Java ecosystem with a very well-defined


		## Usage at the command line

		Maven divides its concept of a build into a "lifecycle" of a linear sequence of steps, called "phases".


		where `<projectlist>` is a comma-separated list of names of the subfolders which contain the submodule configuration pom.xml files.

		If you want to run only a specific test(s), use the `-Dtest="<test>"` option, where `<test>` can be a test name, a class name / package prefix, or a comma-separate list of them.

ICU-22324 Add docs for Maven usage #2655

ICU-22324 Add docs for Maven usage #2655

Conversation

echeran commented Oct 3, 2023 • edited

Checklist

jira-pull-request-webhook bot commented Oct 4, 2023

markusicu left a comment

Choose a reason for hiding this comment

jira-pull-request-webhook bot commented Oct 4, 2023

mihnita Oct 5, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mihnita Oct 5, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mihnita left a comment

Choose a reason for hiding this comment

mihnita left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mihnita left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jira-pull-request-webhook bot commented Oct 5, 2023

echeran commented Oct 3, 2023 •

edited

mihnita Oct 5, 2023 •

edited

mihnita Oct 5, 2023 •

edited