vpelikh commented on PR #4230: URL: https://github.com/apache/logging-log4j2/pull/4230#issuecomment-5748645675
Thanks @ramanathan1504 for the review. I've addressed the comments in two commits. Below is a point-by-point rundown. ## 1. `appenders.adoc` / `filters.adoc` — redo from `main` (+ only `#2696` changes) > Flume, JMS, Kafka, JPA, SMTP and JeroMQ appenders are not in 3.x, and Failover, Rewrite, Http and RollingFile are already in `appenders/*.adoc`. Can this page be `main`'s version plus only the #2696 changes? Both pages are now restored from `main` (diff vs `main` is empty). The ported 2.x single-page content is gone: the Flume / JMS / Kafka / JPA / SMTP / JeroMQ appenders that 3.x doesn't ship, and the duplication of `appenders/*.adoc`. The `filters.adoc` inline-anchor duplication (the extra `== Collection`) is removed too. ## 2. `nav.adoc` — drop the `thanks.adoc` entry Dropped the `** xref:thanks.adoc[Thanks]` line from `nav.adoc`. ## 3. `configuration.adoc` — Log4j 1 config warning / missing `migration.adoc` > `manual/migration.adoc` does not exist on `main`, and Log4j 3 has no Log4j 1 config support. Removed the `[WARNING]` block in `configuration.adoc` that referenced `manual/migration.adoc#ConfigurationCompatibility`. Log4j 3 has no Log4j 1 config support, so the block (and its dead xref) is gone. ## 4. `@Order` vs `@Ordered` on `ConfigurationFactory` plugins > `OrderedComparator` sorts `@Ordered` smallest first and `Ordered.FIRST` is `Integer.MIN_VALUE`, but here it sorts largest first. So `@Ordered(Ordered.FIRST)` on a factory ends up last. Should factories keep `@Order`, or should this comparator follow the `@Ordered` direction? A test in `log4j-core-test` for either answer would pin it. cc @jvz This PR keeps the configuration factories on `@Ordered` and adds `OrderComparatorTest` in `log4j-core-test` pinning the current behavior. Note that `OrderComparator` reads both `@Order` and `@Ordered` and still sorts in descending order (larger value = higher priority), so `@Ordered(Ordered.FIRST)` (`Integer.MIN_VALUE`) still ends up last — that matches the pre-existing `@Order` semantics and the "descending order" documented for the predefined `ConfigurationFactory` plugins, but it diverges from the DI `OrderedComparator` (ascending, `FIRST` first). If the maintainers would rather have `OrderComparator` follow the `@Ordered` ascending convention instead, that's a small change to the comparator plus flipping the new `OrderComparatorTest`. ## 5. Changelog / port rule (#3161) Added `src/changelog/.3.x.x/2696_manual_revamp.xml` linking the original `#2696` port, per the `#3161` port-format / changelog convention. ## 6. Website build / broken xrefs I ran the Antora website build locally. It surfaces only 4 broken-xref targets (`jakarta.adoc`, `logsep.adoc`, `log4j-docker.adoc`, `customloglevels.adoc`), all of which are **pre-existing on `main`** (they come from `main`'s versions of `appenders.adoc`, `architecture.adoc`, `cloud.adoc`, and `filters.adoc`). This port introduces **no new broken xrefs**; the previously-broken ones in the ported single-page content (e.g. the `migration.adoc` reference) are gone. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
