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

fix: static file support route_prefix #835

Merged
merged 3 commits into from
Jul 28, 2022

Conversation

solarknight
Copy link

Use purely random route for static files if --random-route enabled, otherwise add route_prefix before generated route

@svenstaro
Copy link
Owner

Thanks! Any chance you could also make sure this is tested by one of the tests? Otherwise we might get regressions again.

@solarknight
Copy link
Author

Thanks! Any chance you could also make sure this is tested by one of the tests? Otherwise we might get regressions again.
Oh sorry, I'll check the tests :)

@svenstaro
Copy link
Owner

Also check out the CI which failed due to some formatting problem. :)

@svenstaro
Copy link
Owner

svenstaro commented Jul 18, 2022

@solarknight Hey, you still looking into this?

@solarknight solarknight force-pushed the fix/css_route_prefix branch from 5d41dd8 to cc274ee Compare July 20, 2022 10:11
@solarknight
Copy link
Author

solarknight commented Jul 20, 2022

@solarknight Hey, you still looking into this?

Hey, finally I complete the test for static file route check (really a newbie to rust).
Unfortunately I use push -f by mistake. Do you mind if I make a new PR, or any other suggestions?

@svenstaro
Copy link
Owner

Could you rebase this so the history becomes tidier?

tests/config.rs Outdated
use rstest::rstest;
use select::{document::Document, predicate::Attr};

#[rstest]
Copy link
Owner

Choose a reason for hiding this comment

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

Can you stick this test into serve_request.rs?

Copy link
Author

Choose a reason for hiding this comment

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

Done, rename the test to keep consistent with existing tests.

@solarknight solarknight force-pushed the fix/css_route_prefix branch from cc274ee to e571258 Compare July 28, 2022 03:51
@solarknight
Copy link
Author

@svenstaro All above suggestions should be applied, please review the changes.

@svenstaro svenstaro merged commit bcfcf1e into svenstaro:master Jul 28, 2022
@svenstaro
Copy link
Owner

Very cool, thanks!

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.

2 participants