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.

Sign up for GitHub

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

Jump to bottom

feat: Support Reactive Routes #1133

Merged
merged 1 commit into from
Sep 4, 2023
Merged

feat: Support Reactive Routes #1133

merged 1 commit into from
Sep 4, 2023

Conversation

angelozerr
Copy link
Contributor

feat: Support Reactive Routes

Fixes #1114

@fbricon
Copy link
Contributor

fbricon commented Aug 30, 2023

we should have unit tests for that

@angelozerr
Copy link
Contributor Author

angelozerr commented Aug 31, 2023 •
edited
Loading

we should have unit tests for that

Yes sure. I have implemented quickly the support and now I need to clean my code and write tests.

@gastaldi here the result:

  • classic usecase:

image

  • routebase usecase:

image

  • mutiple route usecase:

image

@angelozerr angelozerr force-pushed the codelens-reactive-route branch 2 times, most recently from 3cffa6e to 19a1d7f Compare August 31, 2023 09:22
@angelozerr
Copy link
Contributor Author

My code is clean now, I need to write tests but @fbricon @gastaldi you can start to review it

@fbricon
Copy link
Contributor

fbricon commented Aug 31, 2023

  • mutiple route usecase:

image

I wonder if we should have 1 URL per line. How many different routes can a user add, realistically? @gastaldi WDYT?

@gastaldi
Copy link
Contributor

@fbricon in my experience, a single @Route is enough per method

@angelozerr angelozerr force-pushed the codelens-reactive-route branch 2 times, most recently from 6d53a63 to 64d626a Compare September 1, 2023 08:32
@angelozerr
Copy link
Contributor Author

angelozerr commented Sep 1, 2023 •
edited
Loading

I support now implicit usage when a method is annotated with Route:

image

@angelozerr
Copy link
Contributor Author

I wonder if we should have 1 URL per line. How many different routes can a user add, realistically? @gastaldi WDYT?

I fear that we takes some times to do implement that for few usages. if you want to have that really, we could do that in a separate PR.

@angelozerr angelozerr force-pushed the codelens-reactive-route branch 2 times, most recently from 941f22e to 41cc367 Compare September 1, 2023 16:30
@angelozerr angelozerr marked this pull request as ready for review September 1, 2023 16:31
@angelozerr
Copy link
Contributor Author

Now tests exist.

@sonarcloud
Copy link

sonarcloud bot commented Sep 4, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@fbricon fbricon merged commit 2116391 into redhat-developer:main Sep 4, 2023
12 checks passed
@fbricon
Copy link
Contributor

fbricon commented Sep 4, 2023

Thanks @angelozerr

@angelozerr angelozerr mentioned this pull request Sep 4, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fbricon fbricon fbricon left review comments

@gastaldi gastaldi Awaiting requested review from gastaldi

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Support Reactive Routes
3 participants
@angelozerr @fbricon @gastaldi