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

Add missing @PathParam value in examples #20006

Merged
merged 1 commit into from
Oct 11, 2021

Conversation

glefloch
Copy link
Member

@glefloch glefloch commented Sep 8, 2021

This adds missing value on @PathParam annotation examples.

@geoand
Copy link
Contributor

geoand commented Sep 8, 2021

These aren't really missing, but they imply that org.jboss.resteasy.annotations.jaxrs.PathParam is being used

@glefloch
Copy link
Member Author

glefloch commented Sep 8, 2021

ok, should we specify it somehow ? or keep it as is ?

@geoand
Copy link
Contributor

geoand commented Sep 8, 2021

We can include the full class name, that would make it unambiguous.

However, we are likely going to move to RESTEasy Reactive as the default, and there the annotation has a different name.
So perhaps let's hold off on this.

@@ -287,7 +287,7 @@ public class GreetingResource {
@GET
@Produces(MediaType.TEXT_PLAIN)
@Path("/greeting/{name}")
public String greeting(@PathParam String name) {
public String greeting(@PathParam("name") String name) {
Copy link
Member

Choose a reason for hiding this comment

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

Nope. We are using org.jboss.resteasy.annotations.jaxrs.PathParam, not the JAX-RS @PathParam and the RESTEasy one doesn't require the name.

And if something needs fixing, it would be making sure the imports are always there so that's it's clear. Typically, in this case, the imports are there.

Copy link
Member Author

Choose a reason for hiding this comment

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

they are almost present everywhere, I will add the missing ones.

@glefloch
Copy link
Member Author

@gsmet I added missing imports, should we merge this?

@gsmet gsmet merged commit 60871e6 into quarkusio:main Oct 11, 2021
@quarkus-bot quarkus-bot bot added this to the 2.5 - main milestone Oct 11, 2021
@aloubyansky aloubyansky modified the milestones: 2.5 - main, 2.4.0.Final Oct 18, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants