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
[WFLY-5656]: No warning is shown when deploying web application with defined valves. #8407
Conversation
@@ -74,6 +74,11 @@ public void deploy(DeploymentPhaseContext phaseContext) throws DeploymentUnitPro | |||
warMetaData.setJBossWebMetaData(jBossWebMetaData); | |||
// if the jboss-web.xml has a distinct-name configured, then attach the value to this | |||
// deployment unit | |||
if(jBossWebMetaData.getValves() != null) { | |||
jBossWebMetaData.getValves().stream().forEach((valve) -> { |
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 real overkill for iterating a list :)
for (Valve valve: jBossWebMetaData.getValves()) would do just fine :)
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.
I'm a hipster baby ;)
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.
Yuck. :)
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.
At the very least, note that Iterable
has forEach
as well, so you don't really have to create a Stream
. Though I'd personally agree with Tomaz 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.
Benchmark: How Java 8 Lambdas and Streams Can Make Your Code 5 Times Slower: http://blog.takipi.com/benchmark-how-java-8-lambdas-and-streams-can-make-your-code-5-times-slower/
@stuartwdouglas Please approve, and ack the JBEAP if you think it should be acked. |
…defined valves. Adding warning traces if valves are defined.
Linux Build 8991 outcome was FAILURE using a merge of c2ea625 Failed tests
|
retest this please |
[WFLY-5656]: No warning is shown when deploying web application with defined valves.
Adding warning traces if valves are defined.
Jira: https://issues.jboss.org/browse/WFLY-5656
https://issues.jboss.org/browse/JBEAP-1865