-
Notifications
You must be signed in to change notification settings - Fork 60
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.
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
feat(462): add war/jar timings and advise to exclude unchanged jars/wars #463
feat(462): add war/jar timings and advise to exclude unchanged jars/wars #463
Conversation
Signed-off-by: Jean-Louis Monteiro <jlmonteiro@tomitribe.com>
org.eclipse.transformer/src/main/java/org/eclipse/transformer/action/impl/ActionImpl.java
Outdated
Show resolved
Hide resolved
org.eclipse.transformer/src/main/java/org/eclipse/transformer/action/impl/ChangesImpl.java
Outdated
Show resolved
Hide resolved
org.eclipse.transformer/src/main/java/org/eclipse/transformer/action/impl/ChangesImpl.java
Outdated
Show resolved
Hide resolved
org.eclipse.transformer/src/main/java/org/eclipse/transformer/action/impl/ChangesImpl.java
Outdated
Show resolved
Hide resolved
- upgrade to Java 11 to use Duration instead of own computation - move code from the generic ActionImpl to ZipActionImpl - remove 'ms' unit from log because it's using HH:MM:SS Signed-off-by: Jean-Louis Monteiro <jlmonteiro@tomitribe.com>
@bjhargrave How does it look now? |
@@ -418,6 +433,25 @@ private void applyZipStream( | |||
} | |||
} | |||
|
|||
private void printZipActionDuration(final String inputName) { | |||
if (getLogger().isInfoEnabled()) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ActionImpl reports this for DEBUG level logging and we should be consistent here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes I know, but that is exactly the purpose of the issue #462 and this PR.
Debug mode produces too much such as it's not usable and slows down the whole transformation process.
} | ||
|
||
private void printAdviseOnUnchanged(final String inputName) { | ||
if (getLogger().isInfoEnabled() && !getActiveChanges().isChanged()) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the value of this? I think we already report the number of changes in an element which may be zero thus providing this information.
Also, the advice is rather specific to a specific artifact when transformation rules should be more general to the domain. Updating the artifact to a later version without removing the advised exclusions can result in a partial transformation of the artifact if the excluded element in the newer artifact does indeed require transformation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a valid point. Of course if you change the rules, you should always run without any exclusions. But if you do iterations, or if you start converting some of your applications dependencies to jakarta, you can safely exclude them from the process.
Signed-off-by: Jean-Louis Monteiro <jlmonteiro@tomitribe.com>
org.eclipse.transformer/src/main/java/org/eclipse/transformer/action/impl/ZipActionImpl.java
Outdated
Show resolved
Hide resolved
Signed-off-by: Jean-Louis Monteiro <jlmonteiro@tomitribe.com>
Creating new PRs because it seems it complains with the sign-off check |
No description provided.