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

Added prefix and route nesting to AppRoutes #1241

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

DenuxPlays
Copy link
Contributor

@DenuxPlays DenuxPlays commented Feb 3, 2025

replaced snapshot testing with normal asserts:

It was faster to replace snapshots with basic asserts then to look through multiple snapshot files with no relevant content.
Hope thats ok but I could revert it.

Closes: #1116

@DenuxPlays DenuxPlays force-pushed the feature/nesting-routes branch from e40e529 to 348835f Compare February 3, 2025 17:41
@DenuxPlays
Copy link
Contributor Author

@kaplanelad
That is what I had in mind.
Can you check if this is what you want.

I am open to suggestions

- replaced snapshot testing with normal asserts
@DenuxPlays DenuxPlays force-pushed the feature/nesting-routes branch 3 times, most recently from 5eea60a to b37e6d9 Compare February 3, 2025 20:03
@DenuxPlays DenuxPlays force-pushed the feature/nesting-routes branch from b37e6d9 to 08ba4ab Compare February 3, 2025 20:15
@DenuxPlays
Copy link
Contributor Author

Finally fixed the ci

It's now ready for review :)

src/controller/app_routes.rs Outdated Show resolved Hide resolved
src/controller/app_routes.rs Outdated Show resolved Hide resolved
src/controller/app_routes.rs Outdated Show resolved Hide resolved
src/controller/app_routes.rs Outdated Show resolved Hide resolved
src/controller/app_routes.rs Outdated Show resolved Hide resolved
@DenuxPlays DenuxPlays requested a review from kaplanelad February 5, 2025 16:49
@DenuxPlays DenuxPlays force-pushed the feature/nesting-routes branch from f1fb0a2 to 9468fe6 Compare February 5, 2025 19:56
@DenuxPlays
Copy link
Contributor Author

DenuxPlays commented Feb 5, 2025

I think I fixed the doc test.

I've added the ignore attribute.
Not sure if this is what we want as it is not exactly recommended but compile_fail isn't a good fit either.

If you want you can take a look here to see what attributes are available.

@kaplanelad
Copy link
Contributor

I think I fixed the doc test.

I've added the ignore attribute. Not sure if this is what we want as it is not exactly recommended but compile_fail isn't a good fit either.

If you want you can take a look here to see what attributes are available.

No, this isn’t what we want—this was actually my original comment 🙂.
Let me take a look and push a fix to your PR.

@kaplanelad
Copy link
Contributor

The ignore feature has been removed: b4aa3f6.

Also, I noticed that we don’t have documentation on AppRoutes. Could you please add it here?: https://loco.rs/docs/the-app/controller/

@DenuxPlays
Copy link
Contributor Author

Added some docs

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Unable to nest prefixes/routes
2 participants