Skip to content
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

Bump Dekorate to 3.4.1 #31445

Merged
merged 1 commit into from Feb 28, 2023
Merged

Bump Dekorate to 3.4.1 #31445

merged 1 commit into from Feb 28, 2023

Conversation

Sgitario
Copy link
Contributor

@quarkus-bot quarkus-bot bot added the area/dependencies Pull requests that update a dependency file label Feb 27, 2023
@gsmet
Copy link
Member

gsmet commented Feb 27, 2023

@Sgitario I'm not sure how often isVerbose was called but executing the call only once will be enough to remove all the additional cost we added?
I'm a bit worried that even one time could be too much compared to the situation we had before but you're the expert here, I'm just asking questions :).

@Sgitario
Copy link
Contributor Author

@Sgitario I'm not sure how often isVerbose was called but executing the call only once will be enough to remove all the additional cost we added? I'm a bit worried that even one time could be too much compared to the situation we had before but you're the expert here, I'm just asking questions :).

The problem is that we were calling too many times to the isVerbose method in the Topological Sort algorithm (which is new in Dekorate 3.4.0) and every time we invoked this method we were checking this property via the configuration registry (I wasn't aware of the cost of this call :/ ).

After these changes, the value of isVerbose will be evaluated just once, and taking into account we are also using the isVerbose method in Dekorate core, when it's used by the Topological Sort algorithm, the cost will be free.

@gsmet
Copy link
Member

gsmet commented Feb 27, 2023

OK, thanks for the clarification.

@punkratz312
Copy link

@gsmet is there a reason why dependabot is not bumping the versions. these mr´s are very important. thanks for the contribution. on github the bot in actually included i think the config file need´s an adjustment, then it should work here as well?
image

@quarkus-bot

This comment has been minimized.

@gsmet
Copy link
Member

gsmet commented Feb 28, 2023

@Sgitario the failures were unrelated. (I need to understand what is going on with these tests as they are always failing now)

@quarkus-bot
Copy link

quarkus-bot bot commented Feb 28, 2023

Failing Jobs - Building 863583b

Status Name Step Failures Logs Raw logs
JVM Tests - JDK 11 Build Failures Logs Raw logs
JVM Tests - JDK 11 Windows Build ⚠️ Check → Logs Raw logs
✔️ JVM Tests - JDK 17
✔️ JVM Tests - JDK 19
Maven Tests - JDK 11 Build Failures Logs Raw logs
Maven Tests - JDK 11 Windows Build Failures Logs Raw logs
Native Tests - Security1 Build ⚠️ Check → Logs Raw logs

Full information is available in the Build summary check run.

Failures

⚙️ JVM Tests - JDK 11 #

- Failing: integration-tests/kafka-snappy 

📦 integration-tests/kafka-snappy

io.quarkus.it.kafka.KafkaSnappyConsumerTest. - More details - Source on GitHub

java.lang.RuntimeException: java.lang.reflect.InvocationTargetException
	at io.quarkus.test.junit.QuarkusTestExtension.throwBootFailureException(QuarkusTestExtension.java:625)
	at io.quarkus.test.junit.QuarkusTestExtension.interceptBeforeAllMethod(QuarkusTestExtension.java:680)

⚙️ Maven Tests - JDK 11 #

- Failing: integration-tests/maven 

📦 integration-tests/maven

io.quarkus.maven.it.BuildIT.testClassLoaderLinkageError line 102 - More details - Source on GitHub

org.opentest4j.AssertionFailedError: 

expected: 0

io.quarkus.maven.it.BuildIT.testClassLoaderLinkageError line 102 - More details - Source on GitHub

org.opentest4j.AssertionFailedError: 

expected: 0

⚙️ Maven Tests - JDK 11 Windows #

- Failing: integration-tests/maven 

📦 integration-tests/maven

io.quarkus.maven.it.BuildIT.testClassLoaderLinkageError line 102 - More details - Source on GitHub

org.opentest4j.AssertionFailedError: 

expected: 0

io.quarkus.maven.it.BuildIT.testClassLoaderLinkageError line 102 - More details - Source on GitHub

org.opentest4j.AssertionFailedError: 

expected: 0

@gsmet gsmet merged commit 0f7fd90 into quarkusio:main Feb 28, 2023
@quarkus-bot quarkus-bot bot added this to the 3.0 - main milestone Feb 28, 2023
@Sgitario Sgitario deleted the bump_dekorate branch February 28, 2023 12:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/dependencies Pull requests that update a dependency file
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants