sparckles / Robyn

Robyn is a Super Fast Async Python Web Framework with a Rust runtime.
https://robyn.tech/
BSD 2-Clause "Simplified" License
3.9k stars 198 forks source link

fix: trailing slash issue #819

Closed VishnuSanal closed 1 month ago

VishnuSanal commented 1 month ago

Description

This PR fixes #818

@sansyrox PTAL :)

vercel[bot] commented 1 month ago

@VishnuSanal is attempting to deploy a commit to the sparckles Team on Vercel.

A member of the Team first needs to authorize it.

sansyrox commented 1 month ago

Hey @VishnuSanal 👋

Thanks for the PR, but I see no changes in the PR. Could you have a look again?

VishnuSanal commented 1 month ago

Hey @VishnuSanal 👋

Thanks for the PR, but I see no changes in the PR. Could you have a look again?

ouch!! that lost somewhere whilst fixing the CI. :D PTAL now :)

sansyrox commented 1 month ago

Hey @VishnuSanal

I have made some changes and have added a few suggestions. Do let me know what you think 😄

sansyrox commented 1 month ago

Also, @VishnuSanal , please add an integration test. Add a route in integration_tests/base_routes.py and then make a curl request on both the routes like this - https://github.com/sparckles/Robyn/blob/main/integration_tests/test_get_requests.py

VishnuSanal commented 1 month ago

please add an integration test. Add a route in integration_tests/base_routes.py and then make a curl request on both the routes like this - https://github.com/sparckles/Robyn/blob/main/integration_tests/test_get_requests.py

@sansyrox why add a new route? also, why integration_tests/test_get_requests.py? wouldn't modifying integration_tests/test_basic_routes.py like this suffice? 🤔

sansyrox commented 1 month ago

@VishnuSanal , we tend to add one route for its function. If you explore the file, you will find out that we have one dedicated route for get requests, one for posts, etc.

why add a new route? also, why integration_tests/test_get_requests.py? wouldn't modifying integration_tests/test_basic_routes.py like

vercel[bot] commented 1 month ago

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
robyn ✅ Ready (Inspect) Visit Preview 💬 Add feedback May 19, 2024 5:44pm
codspeed-hq[bot] commented 1 month ago

CodSpeed Performance Report

Merging #819 will not alter performance

Comparing VishnuSanal:main (a773240) with main (00ca522)

Summary

✅ 108 untouched benchmarks

🆕 1 new benchmarks

Benchmarks breakdown

Benchmark main VishnuSanal:main Change
🆕 test_trailing_slash N/A 46.6 ms N/A
sansyrox commented 1 month ago

@VishnuSanal , the ci is failing on formatting. Can you please fix that?

sansyrox commented 1 month ago

All looks good to merge otherwise 😄

sansyrox commented 1 month ago

Great work @VishnuSanal 😄