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

Rename webjars-locator to web-dependency-locator #40066

Merged
merged 1 commit into from
Apr 22, 2024

Conversation

phillip-kruger
Copy link
Member

As discussed, we rename (and relocate) the webjars-locator to something more generic, like web-dependency-locator, as now we support mvnpm and importmaps

@phillip-kruger phillip-kruger requested a review from gsmet April 15, 2024 06:06
@quarkus-bot quarkus-bot bot added area/dependencies Pull requests that update a dependency file area/documentation labels Apr 15, 2024

This comment has been minimized.

@phillip-kruger phillip-kruger requested a review from ia3andy April 15, 2024 06:21
@quarkus-bot quarkus-bot bot added the area/devtools Issues/PR related to maven, gradle, platform and cli tooling/plugins label Apr 15, 2024

This comment has been minimized.

Copy link

github-actions bot commented Apr 15, 2024

🙈 The PR is closed and the preview is expired.

This comment has been minimized.

Copy link
Member

@gsmet gsmet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! I added a few comments.

@ia3andy
Copy link
Contributor

ia3andy commented Apr 15, 2024

This needs to wait, we are discussing using this opportunity to move it to the quarkiverse. https://quarkusio.zulipchat.com/#narrow/stream/191168-core-team/topic/Quarkus.20Web/near/433256889

We are also discussing making it the way (opt-in via extension) to enable serving of mvnpm and webjars which is currently part of vert.x http.

@phillip-kruger
Copy link
Member Author

@gsmet - please check again, I addressed your comments.
@ia3andy - It's not this extension that allows loading from META-INF. This extension only maps the versions path to the non-versioned path, and now also produce a importmap.

We can probably move that logic to this extension. I would assume it's in vertx-http at the moment. That would not change if this extension is in Quarkiverse or here.

My suggestion is we merge here and then see if it will be possible to move that logic out of vertx-http and what the effort and impact would be.

@phillip-kruger
Copy link
Member Author

Things that will break is swagger-ui and graphql-ui.

This comment has been minimized.

This comment has been minimized.

@ia3andy
Copy link
Contributor

ia3andy commented Apr 16, 2024

Things that will break is swagger-ui and graphql-ui.

We might be able to keep the compat through builditems if we keep the logic in vertx http, but add the enablement through builditems.

This way the web-dependency extension can use this SPI for all the detected web dependencies and swagger-ui and graphql-ui can only add the required ones.

This comment has been minimized.

@gsmet
Copy link
Member

gsmet commented Apr 16, 2024

Is there a reason why we would want to move it to the Quarkiverse?

It's useful and small and is already there so, except if we have a good reason to move it, I would keep it around.

@phillip-kruger
Copy link
Member Author

I agree @gsmet

@ia3andy
Copy link
Contributor

ia3andy commented Apr 16, 2024

it was suggested by @cescoffier. I think it belongs more in quarkiverse and while renaming that might be a good opportunity. I have no strong opinion on it.

@cescoffier
Copy link
Member

The reason is just to reduce the size of the core and provide independent release cycles.

@phillip-kruger
Copy link
Member Author

@gsmet you make a call. It would be good to get this into 3.10

@gsmet
Copy link
Member

gsmet commented Apr 17, 2024

It's a bit late for 3.10 unfortunately given we already pushed CR1.

Personally, I would keep this one in core. There are several other extensions I would rather move.

@phillip-kruger
Copy link
Member Author

Can this go into a patch release ? 3.10.1 ?

Copy link

quarkus-bot bot commented Apr 18, 2024

Status for workflow Quarkus Documentation CI

This is the status report for running Quarkus Documentation CI on commit 7f695ff.

✅ The latest workflow run for the pull request has completed successfully.

It should be safe to merge provided you have a look at the other checks in the summary.

⚠️ There are other workflow runs running, you probably need to wait for their status before merging.

Copy link

quarkus-bot bot commented Apr 18, 2024

Status for workflow Quarkus CI

This is the status report for running Quarkus CI on commit 7f695ff.

Failing Jobs

Status Name Step Failures Logs Raw logs Build scan
Native Tests - DevTools Integration Tests Download .m2/repository/io/quarkus ⚠️ Check → Logs Raw logs 🚧
Native Tests - Security2 Upload build reports ⚠️ Check → Logs Raw logs 🔍

You can consult the Develocity build scans.


Flaky tests - Develocity

⚙️ JVM Tests - JDK 21

📦 extensions/smallrye-reactive-messaging-kafka/deployment

io.quarkus.smallrye.reactivemessaging.kafka.deployment.dev.KafkaDevServicesDevModeTestCase.sseStream - History

  • Assertion condition defined as a Lambda expression in io.quarkus.smallrye.reactivemessaging.kafka.deployment.dev.KafkaDevServicesDevModeTestCase Expecting size of: [] to be greater than or equal to 2 but was 0 within 10 seconds. - org.awaitility.core.ConditionTimeoutException
org.awaitility.core.ConditionTimeoutException: 
Assertion condition defined as a Lambda expression in io.quarkus.smallrye.reactivemessaging.kafka.deployment.dev.KafkaDevServicesDevModeTestCase 
Expecting size of:
  []
to be greater than or equal to 2 but was 0 within 10 seconds.
	at org.awaitility.core.ConditionAwaiter.await(ConditionAwaiter.java:167)
	at org.awaitility.core.AssertionCondition.await(AssertionCondition.java:119)
	at org.awaitility.core.AssertionCondition.await(AssertionCondition.java:31)

@gsmet gsmet merged commit 824234e into quarkusio:main Apr 22, 2024
52 of 54 checks passed
@quarkus-bot quarkus-bot bot added this to the 3.11 - main milestone Apr 22, 2024
@gsmet
Copy link
Member

gsmet commented Apr 22, 2024

Can this go into a patch release ? 3.10.1 ?

No we can't do that. It will have to wait for the next one.

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 area/devtools Issues/PR related to maven, gradle, platform and cli tooling/plugins area/documentation triage/flaky-test
Projects
Development

Successfully merging this pull request may close these issues.

4 participants