You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@maven.apache.org by GitBox <gi...@apache.org> on 2022/05/20 12:58:53 UTC
[GitHub] [maven-scm] cstamas opened a new pull request, #142: [SCM-979] ScmLogger nonsense dropped
cstamas opened a new pull request, #142:
URL: https://github.com/apache/maven-scm/pull/142
Using plain Slf4j logging. Anyone integrating
SCM should provide Slf4j implementation and
configure it as he likes.
--
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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-scm] michael-o commented on pull request #142: [SCM-979] ScmLogger nonsense dropped
Posted by GitBox <gi...@apache.org>.
michael-o commented on PR #142:
URL: https://github.com/apache/maven-scm/pull/142#issuecomment-1132910707
> Curious: Why SLF4J instead of the Log4j2-api?
Our entire ecosystem is build around SLF4J
> Essentially the same (logging facade) with 2 advantages:
>
> * Better process (SLF4J has had a period of almost 2 years without any release).
What better process? Full of features most peole don't need? More releases aren't better. SFL4J is just a facade while Log4J2 offery EVERYTHING. There is not much you need to do if the facade is done.
> * More features (also a formatted logger).
Which we don't need, we barely manage to move off string concat.
At the end, we prefer over simplily rather than busload of features.
Log4J2, even as an API, is a no go for me.
--
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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-scm] nielsbasjes commented on pull request #142: [SCM-979] ScmLogger nonsense dropped
Posted by GitBox <gi...@apache.org>.
nielsbasjes commented on PR #142:
URL: https://github.com/apache/maven-scm/pull/142#issuecomment-1132900179
Curious: Why SLF4J instead of the Log4j-api?
Essentially the same (logging facade) with 2 advantages:
- Better process (SLF4J has had a period of almost 2 years without any release).
- More features (also a formatted logger).
--
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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-scm] olamy commented on a diff in pull request #142: [SCM-979] ScmLogger nonsense dropped
Posted by GitBox <gi...@apache.org>.
olamy commented on code in PR #142:
URL: https://github.com/apache/maven-scm/pull/142#discussion_r878842952
##########
maven-scm-providers/maven-scm-provider-hg/src/main/java/org/apache/maven/scm/provider/hg/command/status/HgStatusConsumer.java:
##########
@@ -52,24 +50,24 @@ public void doConsume( ScmFileStatus status, String trimmedLine )
File tmpFile = new File( workingDir, trimmedLine );
if ( !tmpFile.exists() )
{
- if ( getLogger().isInfoEnabled() )
+ if ( logger.isInfoEnabled() )
{
- getLogger().info( "Not a file: " + tmpFile + ". Ignoring" );
+ logger.info( "Not a file: " + tmpFile + ". Ignoring" );
Review Comment:
could be simplify
--
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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-scm] michael-o commented on a diff in pull request #142: [SCM-979] ScmLogger nonsense dropped
Posted by GitBox <gi...@apache.org>.
michael-o commented on code in PR #142:
URL: https://github.com/apache/maven-scm/pull/142#discussion_r878844685
##########
maven-scm-providers/maven-scm-provider-hg/src/main/java/org/apache/maven/scm/provider/hg/command/add/HgAddConsumer.java:
##########
@@ -54,17 +52,17 @@ public void doConsume( ScmFileStatus status, String trimmedLine )
File tmpFile = new File( workingDir, trimmedLine );
if ( !tmpFile.exists() )
{
- if ( getLogger().isWarnEnabled() )
+ if ( logger.isWarnEnabled() )
{
- getLogger().warn( "Not a file: " + tmpFile + ". Ignored" );
+ logger.warn( "Not a file: " + tmpFile + ". Ignored" );
Review Comment:
This PR is a mechanical replacement. Placeholders are next, but are right of course.
--
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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-scm] olamy commented on a diff in pull request #142: [SCM-979] ScmLogger nonsense dropped
Posted by GitBox <gi...@apache.org>.
olamy commented on code in PR #142:
URL: https://github.com/apache/maven-scm/pull/142#discussion_r878842822
##########
maven-scm-providers/maven-scm-provider-hg/src/main/java/org/apache/maven/scm/provider/hg/command/add/HgAddConsumer.java:
##########
@@ -54,17 +52,17 @@ public void doConsume( ScmFileStatus status, String trimmedLine )
File tmpFile = new File( workingDir, trimmedLine );
if ( !tmpFile.exists() )
{
- if ( getLogger().isWarnEnabled() )
+ if ( logger.isWarnEnabled() )
{
- getLogger().warn( "Not a file: " + tmpFile + ". Ignored" );
+ logger.warn( "Not a file: " + tmpFile + ". Ignored" );
Review Comment:
why not changing/simplify this as well by removing the if and `logger.warn( "Not a file: {}. Ignored", tmpFile );`
this apply to plenty of lines
--
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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-scm] asfgit merged pull request #142: [SCM-979] ScmLogger nonsense dropped
Posted by GitBox <gi...@apache.org>.
asfgit merged PR #142:
URL: https://github.com/apache/maven-scm/pull/142
--
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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-scm] cstamas commented on a diff in pull request #142: [SCM-979] ScmLogger nonsense dropped
Posted by GitBox <gi...@apache.org>.
cstamas commented on code in PR #142:
URL: https://github.com/apache/maven-scm/pull/142#discussion_r878259546
##########
maven-scm-api/src/main/java/org/apache/maven/scm/command/AbstractCommand.java:
##########
@@ -35,7 +35,7 @@
public abstract class AbstractCommand
implements Command
{
- private ScmLogger logger = new Slf4jScmLogger( getClass() );
+ protected Logger logger = LoggerFactory.getLogger( getClass() );
Review Comment:
eh, the class hierarchy is really deep, so to make each class has own logger? Meg.
Also, this change above is on par with what we had before: subclasses used getLogger to get logger from parent.... so effect is same sans getter for logger.
--
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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-scm] slawekjaranowski commented on a diff in pull request #142: [SCM-979] ScmLogger nonsense dropped
Posted by GitBox <gi...@apache.org>.
slawekjaranowski commented on code in PR #142:
URL: https://github.com/apache/maven-scm/pull/142#discussion_r878243579
##########
maven-scm-api/src/main/java/org/apache/maven/scm/command/AbstractCommand.java:
##########
@@ -35,7 +35,7 @@
public abstract class AbstractCommand
implements Command
{
- private ScmLogger logger = new Slf4jScmLogger( getClass() );
+ protected Logger logger = LoggerFactory.getLogger( getClass() );
Review Comment:
better is private and to have separate logger for each class, in this way logger name will be inconsistent with usage.
--
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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-scm] olamy commented on a diff in pull request #142: [SCM-979] ScmLogger nonsense dropped
Posted by GitBox <gi...@apache.org>.
olamy commented on code in PR #142:
URL: https://github.com/apache/maven-scm/pull/142#discussion_r878842889
##########
maven-scm-providers/maven-scm-provider-hg/src/main/java/org/apache/maven/scm/provider/hg/command/remove/HgRemoveConsumer.java:
##########
@@ -55,17 +53,17 @@ public void doConsume( ScmFileStatus status, String trimmedLine )
File tmpFile = new File( workingDir, trimmedLine );
if ( !tmpFile.exists() )
{
- if ( getLogger().isWarnEnabled() )
+ if ( logger.isWarnEnabled() )
{
- getLogger().warn( "Not a file: " + tmpFile + ". Ignored" );
+ logger.warn( "Not a file: " + tmpFile + ". Ignored" );
Review Comment:
could be simplify
--
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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org