Skip to content

Reduce amount of log junk emitted #1286

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 4 commits into from
Sep 12, 2025
Merged

Reduce amount of log junk emitted #1286

merged 4 commits into from
Sep 12, 2025

Conversation

elharo
Copy link
Contributor

@elharo elharo commented Aug 6, 2025 •
edited
Loading

Changes a lot of info logs statements to debug

@elharo elharo marked this pull request as ready for review August 6, 2025 20:44
@elharo elharo changed the title Reduce amount of log junk we emit Reduce amount of log junk emitted Aug 6, 2025
@elharo elharo added the enhancement New feature or request label Aug 7, 2025
@elharo elharo requested a review from kwin August 11, 2025 11:10
kwin
kwin requested changes Aug 11, 2025
View reviewed changes
Copy link
Member

@kwin kwin left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Some logs I would leave level on INFO, most adjustments look good to me.

maven-scm-plugin/src/main/java/org/apache/maven/scm/plugin/AddMojo.java Outdated
@@ -54,7 +54,7 @@ public void execute() throws MojoExecutionException {

checkResult(result);

getLog().info("" + result.getAddedFiles().size() + " files successfully added.");
getLog().debug("" + result.getAddedFiles().size() + " files successfully added.");
Copy link
Member

Choose a reason for hiding this comment

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

This I would rather leave with info as this is an important hint as often usage of wildcards leads to unexpected files being added. I think one info per goal is not overly verbose.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

reverted

maven-scm-plugin/src/main/java/org/apache/maven/scm/plugin/CheckLocalModificationsMojo.java Outdated
@@ -61,7 +61,7 @@ public CheckLocalModificationsMojo(ScmManager manager, SettingsDecrypter setting

public void execute() throws MojoExecutionException {
if (skip) {
getLog().info("check-local-modification execution has been skipped");
Copy link
Member

Choose a reason for hiding this comment

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

I think skipping a mojo emits an INFO log also with other mojos, like https://github.com/apache/maven-surefire/blob/98d081e671888831ab16c66f4e02fc7e73802d2f/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/AbstractSurefireMojo.java#L939. I would rather leave on info level.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

reverted

maven-scm-plugin/src/main/java/org/apache/maven/scm/plugin/UpdateMojo.java Outdated

if (project.getProperties() != null) // Remove the test when we'll use plugin-test-harness 1.0-alpha-2
{
project.getProperties().put(revisionKey, revision);
}

getLog().info("Project at revision " + revision);
getLog().debug("Project at revision " + revision);
Copy link
Member

Choose a reason for hiding this comment

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

This is important to leave on INFO for me.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

reverted

@kwin
Copy link
Member

kwin commented Sep 12, 2025

@elharo I would like to do a release soon, can you squash and merge?

@elharo
Copy link
Contributor Author

elharo commented Sep 12, 2025

Tests look broken or at least flaky:

Error:  org.apache.maven.scm.provider.git.gitexe.command.changelog.GitExeChangeLogCommandTckTest.testChangeLogCommand -- Time elapsed: 1.260 s <<< FAILURE!
java.lang.AssertionError: 
The git-log command failed.: /bin/sh -c cd '/Users/runner/work/maven-scm/maven-scm/maven-scm-providers/maven-scm-providers-git/maven-scm-provider-gitexe/target/scm-test/working-copy' && 'git' 'whatchanged' '--format=medium' '--decorate=short' '--raw' '--no-merges' '--date=iso' '--' '.'
'git whatchanged' is nominated for removal.
If you still use this command, please add an extra
option, '--i-still-use-this', on the command line
and let us know you still use it by sending an e-mail
to <git@vger.kernel.org>.  Thanks.
fatal: refusing to run without --i-still-use-this

This is preventing me from merging and should be resolved before cutting a release, whether this PR goes in or not.

@elharo elharo merged commit b008e4c into master Sep 12, 2025
66 of 79 checks passed
@elharo elharo deleted the logger branch September 12, 2025 17:08
@github-actions github-actions bot added this to the 2.2.1 milestone Sep 12, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kwin kwin kwin approved these changes

Assignees

@elharo elharo

Labels
enhancement New feature or request
Projects
None yet
Milestone
2.2.1
Development

Successfully merging this pull request may close these issues.
2 participants
@elharo @kwin