Skip to content

[MNG-7827] Ensure maven 4 Log API is the primary and documented API for mojos#1187

Closed
rmannibucau wants to merge 1 commit intomasterfrom
MNG-7827
Closed

[MNG-7827] Ensure maven 4 Log API is the primary and documented API for mojos#1187
rmannibucau wants to merge 1 commit intomasterfrom
MNG-7827

Conversation

@rmannibucau
Copy link
Copy Markdown
Contributor

@rmannibucau rmannibucau commented Jun 26, 2023

Following this checklist to help us incorporate your
contribution quickly and easily:

  • Make sure there is a JIRA issue filed
    for the change (usually before you start working on it). Trivial changes like typos do not
    require a JIRA issue. Your pull request should address just this issue, without
    pulling in other changes.
  • Each commit in the pull request should have a meaningful subject line and body.
  • Format the pull request title like [MNG-XXX] SUMMARY,
    where you replace MNG-XXX and SUMMARY with the appropriate JIRA issue.
  • Also format the first line of the commit message like [MNG-XXX] SUMMARY.
    Best practice is to use the JIRA issue title in both the pull request title and in the first line of the commit message.
  • Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
  • Run mvn clean verify to make sure basic checks pass. A more thorough check will
    be performed on your pull request automatically.
  • You have run the Core IT successfully.

@gnodet gnodet self-requested a review June 26, 2023 20:53
@gnodet
Copy link
Copy Markdown
Contributor

gnodet commented Jun 27, 2023

@rmannibucau not sure what the point of this PR is. I would not force 3.x plugins to use the maven 4 api, I think it's better to keep it for 4.x plugins.

@rmannibucau
Copy link
Copy Markdown
Contributor Author

@gnodet there are two goals:

  1. Drop the irrelevant documentation we never agreed upon to use SLF4J (api) for plugins
  2. Ensure the default is the actual maven logging system and not stdout/stderr directly for other cases if it is present

v4 API is more a way to do the bridge easily and with less internal efforts (since it is hidden anyway)

@gnodet
Copy link
Copy Markdown
Contributor

gnodet commented Jun 28, 2023

  1. Ensure the default is the actual maven logging system and not stdout/stderr directly for other cases if it is present

I don't understand the actual use case. All mojos called by maven are injected with a proper log already. The SystemStreamLog is really a fallback. In which case would this new code actually be used ?

I'd rather merge #793 at this point

@rmannibucau
Copy link
Copy Markdown
Contributor Author

@gnodet good for me too, I wondered about this point too and if true we should delete SystemStreamLog and just fail instead probably. If useful it means we must forward the fallback to the actual expected logging system behind Log and now it is v4 mecanism thanks new Log API, this was the intent behind this PR but happy to just drop it too cause I share your reasoning.

@cstamas
Copy link
Copy Markdown
Member

cstamas commented Jun 30, 2023

  1. Drop the irrelevant documentation we never agreed upon to use SLF4J (api) for plugins

Then your memory is lossy 😄
See https://lists.apache.org/thread/bh9vr9fwt2cqf3ltg88jm1fh5y0d57b0

Please, stop with this "change the logging..." madness.

@rmannibucau
Copy link
Copy Markdown
Contributor Author

@cstamas this is only one of the 4-5 explaining why we must not leak our default impl and where we didnt find an agreement in terms of api leak yes.
Note we dont want to change the logging, just keep the one we had and fix the bug introduced in the javadoc which doesnt reflects discussion outcomes.
Technically, there are multiple cons leaking slf4j but community wise a vote wouldnt pass but in force so both are bad.
Last is that maven didnt migrate its api to slf4j in plugin - never, we miss several steps to say it is done so current absusive comment is even wrong - so let's fix it, will impact nobody as of today and not mislead mojo dev.

@gnodet
Copy link
Copy Markdown
Contributor

gnodet commented Jul 11, 2023

Closing as duplicate of #793

@gnodet gnodet closed this Jul 11, 2023
@slawekjaranowski slawekjaranowski deleted the MNG-7827 branch June 7, 2025 12:55
@slawekjaranowski
Copy link
Copy Markdown
Member

source branch deleted

@jira-importer
Copy link
Copy Markdown

Resolve #9092

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

Successfully merging this pull request may close these issues.

5 participants