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/06/18 07:54:00 UTC
[GitHub] [maven] hboutemy opened a new pull request, #756: MNG-7501 display relative path to pom.xml
hboutemy opened a new pull request, #756:
URL: https://github.com/apache/maven/pull/756
https://issues.apache.org/jira/browse/MNG-7501
--
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] rmannibucau commented on pull request #756: MNG-7501 display relative path to pom.xml
Posted by GitBox <gi...@apache.org>.
rmannibucau commented on PR #756:
URL: https://github.com/apache/maven/pull/756#issuecomment-1162178246
@cstamas exactly, using easy to get and very convention friendly builds we already hit issues, was the only point.
--
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] rmannibucau commented on pull request #756: MNG-7501 display relative path to pom.xml
Posted by GitBox <gi...@apache.org>.
rmannibucau commented on PR #756:
URL: https://github.com/apache/maven/pull/756#issuecomment-1160759508
Hi guys, a few comments as an user:
. Can be neat to drop pom.xml (when "pom.xml"), takes space without more info
. Can it be an option or -X/logger flag? In several multimodules with multiple hierarchy levels (>=3) projects it is too verbose and breaks the output and in single modules it is useless so not sure it is that generic. Personally a debug line after the "---" line would be neat.
--
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] rmannibucau commented on pull request #756: MNG-7501 display relative path to pom.xml
Posted by GitBox <gi...@apache.org>.
rmannibucau commented on PR #756:
URL: https://github.com/apache/maven/pull/756#issuecomment-1162093910
> I don't understand
Using one logger per concern we get the needed info at will and we dont have the over-verbose output issue anymore so it enables more data injection user could activate at need.
> it seems your original use case is not mine
Can be but the fact this pr can be seen as bothering and does not generally works with maven remains so not sure it is good to get it onboard as this, no?
--
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] michael-o commented on pull request #756: MNG-7501 display relative path to pom.xml
Posted by GitBox <gi...@apache.org>.
michael-o commented on PR #756:
URL: https://github.com/apache/maven/pull/756#issuecomment-1160087272
This works for aggregator and reactor? it also works with `-f`?
--
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] cstamas commented on pull request #756: MNG-7501 display relative path to pom.xml
Posted by GitBox <gi...@apache.org>.
cstamas commented on PR #756:
URL: https://github.com/apache/maven/pull/756#issuecomment-1162709496
> @mthmulders and does this impl solves the issue for quarkus? For me it does not seem so since the printed info is already thread in single threaded build (plugins already list the folder and quarkus uses default descriptor name so it is more than sufficient) and in multithreaded builds it looks wrong cause of the interleaving, so isnt it another example of what I'm explaining?
You mean `mvn -T` builds? As that breaks any log, not only this one. In general, if you want multi-threaded build, use `mvnd`, as IMHO `mvn -T` (that is inferior as code wise as capability wise to mvnd) should be simply not used, maybe even deprecated.
--
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] michael-o commented on pull request #756: MNG-7501 display relative path to pom.xml
Posted by GitBox <gi...@apache.org>.
michael-o commented on PR #756:
URL: https://github.com/apache/maven/pull/756#issuecomment-1159399362
This is option 1 or option 2?
--
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] hboutemy commented on a diff in pull request #756: MNG-7501 display relative path to pom.xml
Posted by GitBox <gi...@apache.org>.
hboutemy commented on code in PR #756:
URL: https://github.com/apache/maven/pull/756#discussion_r900808391
##########
maven-embedder/src/test/java/org/apache/maven/cli/event/ExecutionEventLoggerTest.java:
##########
@@ -73,6 +83,7 @@ public void testProjectStarted()
inOrder.verify( logger ).info( "" );
inOrder.verify( logger ).info( "------------------< org.apache.maven:maven-embedder >-------------------" );
inOrder.verify( logger ).info( "Building Apache Maven Embedder 3.5.4-SNAPSHOT" );
+ inOrder.verify( logger ).info( " maven-embedder/pom.xml" );
Review Comment:
I love "Building xxx \n from yyy"
and it adds a clear reason to indentation to match "Building" length, not taste (that inherently varies between people)
--
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] rmannibucau commented on pull request #756: MNG-7501 display relative path to pom.xml
Posted by GitBox <gi...@apache.org>.
rmannibucau commented on PR #756:
URL: https://github.com/apache/maven/pull/756#issuecomment-1162714124
@cstamas no, mvnd log interactive and cool but on CI - where -T is very important - you just get the same than plain mvn and, exactly as for gradle, you don't want the daemon on CI too. So, IMHO, we are in a state where 1. we get more verbose without reasons (single module, multi modules with correct naming artifactid/name), 2. we don't solve the issue in half o the use case (multithreaded, mvn or mvnd) and 3. the info is already present (resources, compiler, etc plugins).
That said you make me think of a good solution ot get that exact solution without breaking all the mentionned use case: just add a plugin which prints a configured string from the project, will be equivalent quite easily to this impl without touching to maven core and not add broken cases. Would it work? Strictly speaking using eval goal on project.bsaedir or project.file *already* solves that.
--
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] michael-o commented on pull request #756: MNG-7501 display relative path to pom.xml
Posted by GitBox <gi...@apache.org>.
michael-o commented on PR #756:
URL: https://github.com/apache/maven/pull/756#issuecomment-1160754664
> 4 spaces aligns "Building xxx" with " from yyy"
>
> ```
> Building Name
> from pom.xml
> ```
>
> to me this alignment is nice
I won't object, this is purely a matter of taste. Just try two and see whether you feel a difference in the output. 4 looks too stray 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] hboutemy commented on pull request #756: MNG-7501 display relative path to pom.xml
Posted by GitBox <gi...@apache.org>.
hboutemy commented on PR #756:
URL: https://github.com/apache/maven/pull/756#issuecomment-1160753279
4 spaces aligns "Building xxx" with " from yyy"
```
Building Name
from pom.xml
```
to me this alignment is nice
--
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] michael-o commented on a diff in pull request #756: MNG-7501 display relative path to pom.xml
Posted by GitBox <gi...@apache.org>.
michael-o commented on code in PR #756:
URL: https://github.com/apache/maven/pull/756#discussion_r903305026
##########
maven-embedder/src/test/java/org/apache/maven/cli/event/ExecutionEventLoggerTest.java:
##########
@@ -119,16 +121,12 @@ public void testProjectStartedOverflow()
inOrder.verify( logger ).info( "" );
inOrder.verify( logger ).info( "--< org.apache.maven.plugins.overflow:maven-project-info-reports-plugin >--" );
inOrder.verify( logger ).info( "Building Apache Maven Project Info Reports Plugin 3.0.0-SNAPSHOT" );
- inOrder.verify( logger ).info( " from pom.xml" );
+ inOrder.verify( logger ).info( adaptDirSeparator( " from pom.xml" ) );
Review Comment:
From a cleanliness perspective I would apply this only to "pom.xml" and not to "from ".
--
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] michael-o commented on pull request #756: MNG-7501 display relative path to pom.xml
Posted by GitBox <gi...@apache.org>.
michael-o commented on PR #756:
URL: https://github.com/apache/maven/pull/756#issuecomment-1160764790
> Hi guys, a few comments as an user:
>
> . Can be neat to drop pom.xml (when "pom.xml"), takes space without more info
You mean show dir only?
> . Can it be an option or -X/logger flag? In several multimodules with multiple hierarchy levels (>=3) projects it is too verbose and breaks the output and in single modules it is useless so not sure it is that generic. Personally a debug line after the "---" line would be neat.
In this case I would recommend the following:
* Normal mode: Use Jansi to obtain terminal width and prepend an ellipsis just like Git's `--stat` does. When wdith cannot be determined use default value (80?)
* Verbose mode: Show full path
WDYT?
--
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] hboutemy commented on pull request #756: MNG-7501 display relative path to pom.xml
Posted by GitBox <gi...@apache.org>.
hboutemy commented on PR #756:
URL: https://github.com/apache/maven/pull/756#issuecomment-1161978062
- drop pom.xml: are we so constrained that we start complexifying?
- mono-module: ok, we can do it, but does it deserve special case?
- debug mode: it defeats the intent = have useful info on normal run (if we run debug mode, we're already lost in details)
- "Generally speaking module name should be sufficient if dev did it right.": no, definitely no, if you have 5 levels of directories and you put them in the module name, you're lost
--
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] hboutemy commented on pull request #756: MNG-7501 display relative path to pom.xml
Posted by GitBox <gi...@apache.org>.
hboutemy commented on PR #756:
URL: https://github.com/apache/maven/pull/756#issuecomment-1162031946
> it is no more true since we get logger configuration on the CLI so we have built-in toggles now :)
I don't understand
> Hmm, do you have some examples?
yes, see the description of the PR
> The side note there can be that all this implementation assumes using a single thread so can not even solve the original use case
it seems your original use case is not mine
--
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] rmannibucau commented on pull request #756: MNG-7501 display relative path to pom.xml
Posted by GitBox <gi...@apache.org>.
rmannibucau commented on PR #756:
URL: https://github.com/apache/maven/pull/756#issuecomment-1160792385
> You mean show dir only?
When descriptor name is the default one yes.
About the ellipsis: it is better but fails to reach its goal for the mentionned projects then so I'd really keep it a debug option.
If goal is to add context I would rather investigate a per log line shortname (unique name for the react based on module/path initials - enough to be unique), solething like `[p/sb/cr][INFO] xxx` for parent/submodule/core for ex.
Generally speaking module name should be sufficient if dev did it right.
Another option could be to support a pattern for the building line but not forcing in default mode it sounds like good to me even if I can see some use case for the some particular env.
Last option I can think about is mvnd one which breaks the output per module.
--
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] rmannibucau commented on pull request #756: MNG-7501 display relative path to pom.xml
Posted by GitBox <gi...@apache.org>.
rmannibucau commented on PR #756:
URL: https://github.com/apache/maven/pull/756#issuecomment-1161988690
> drop pom.xml: are we so constrained that we start complexifying?
Guess that to be fair it should be asked both ways, is current verbosity worth it and holding any information? Less is printed better it is IMHO and complexity is quite relative so think so personally.
> mono-module: ok, we can do it, but does it deserve special case?
Don't think so since the same issue hits big multimodule projects.
> debug mode: it defeats the intent = have useful info on normal run (if we run debug mode, we're already lost in details)
it is no more true since we get logger configuration on the CLI so we have built-in toggles now :)
> "Generally speaking module name should be sufficient if dev did it right.": no, definitely no, if you have 5 levels of directories and you put them in the module name, you're lost
Hmm, do you have some examples? Took tomee, activemq, cxf, camel and it is not the case and it is already complex enough projects IMHO.
The side note there can be that all this implementation assumes using a single thread so can not even solve the original use case (this is where I mentionned putting it per line in compressed form).
--
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] cstamas commented on pull request #756: MNG-7501 display relative path to pom.xml
Posted by GitBox <gi...@apache.org>.
cstamas commented on PR #756:
URL: https://github.com/apache/maven/pull/756#issuecomment-1162170761
> Generally speaking module name should be sufficient if dev did it right.
Please do not be overly smart. Out users has really wildly complex builds, the fact that you tested with tomee and some handful of ASF (OSS, will become important later) project obviously does not mean you tested all out there :smile:
For resolver users were testing on some (closed) projects that were _reported as faster_ with BF than with DF collector, something we with @michael-o were **never able to reproduce** (given the project in question is closed source).
All I want to say: keep it simple instead "smart" and rely on some assumptions. You can always find someone who will outsmart you (in a good or a bad way). And you can always find crazy builds as well.
--
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] rmannibucau commented on pull request #756: MNG-7501 display relative path to pom.xml
Posted by GitBox <gi...@apache.org>.
rmannibucau commented on PR #756:
URL: https://github.com/apache/maven/pull/756#issuecomment-1162707105
@mthmulders and does this impl solves the issue for quarkus? For me it does not seem so since the printed info is already thread in single threaded build (plugins already list the folder and quarkus uses default descriptor name so it is more than sufficient) and in multithreaded builds it looks wrong cause of the interleaving, so isnt it another example of what I'm explaining?
--
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] mthmulders commented on pull request #756: MNG-7501 display relative path to pom.xml
Posted by GitBox <gi...@apache.org>.
mthmulders commented on PR #756:
URL: https://github.com/apache/maven/pull/756#issuecomment-1162698264
> Hmm, do you have some examples? Took tomee, activemq, cxf, camel and it is not the case and it is already complex enough projects IMHO.
I present you the case of [Quarkus](https://github.com/quarkusio/quarkus): their project setup is at least three levels deep and the project has 1027 modules as of today.
--
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] hboutemy merged pull request #756: MNG-7501 display relative path to pom.xml
Posted by GitBox <gi...@apache.org>.
hboutemy merged PR #756:
URL: https://github.com/apache/maven/pull/756
--
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] michael-o commented on a diff in pull request #756: MNG-7501 display relative path to pom.xml
Posted by GitBox <gi...@apache.org>.
michael-o commented on code in PR #756:
URL: https://github.com/apache/maven/pull/756#discussion_r900805758
##########
maven-embedder/src/test/java/org/apache/maven/cli/event/ExecutionEventLoggerTest.java:
##########
@@ -73,6 +83,7 @@ public void testProjectStarted()
inOrder.verify( logger ).info( "" );
inOrder.verify( logger ).info( "------------------< org.apache.maven:maven-embedder >-------------------" );
inOrder.verify( logger ).info( "Building Apache Maven Embedder 3.5.4-SNAPSHOT" );
+ inOrder.verify( logger ).info( " maven-embedder/pom.xml" );
Review Comment:
Seems to be option 1, therefore I would expect rather:
```
Building XXX...
from .../pom.xml
```
Side note: We have inconsistent indentation. Some use two, some four spaces.
--
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