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

Rename params with path_params #460

Merged
merged 12 commits into from
Apr 7, 2023
Merged

Conversation

carlosm27
Copy link
Contributor

Description
This pull request address the proposal to rename params with path_params.

This PR fixes #457

Copy link
Member

@sansyrox sansyrox left a comment

Choose a reason for hiding this comment

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

@carlosm27 , thank you for your PR but you may need to rebase it.

Cargo.toml Outdated Show resolved Hide resolved
@codspeed-hq
Copy link

codspeed-hq bot commented Apr 4, 2023

CodSpeed Performance Report

Merging #460 carlosm27:path_params (8484fe0) will not alter performances.

Summary

🔥 0 improvements
❌ 0 regressions
✅ 74 untouched benchmarks

🆕 0 new benchmarks
⁉️ 0 dropped benchmarks

@sansyrox sansyrox force-pushed the main branch 2 times, most recently from f0bf73a to 97e2c0c Compare April 5, 2023 19:13
Copy link
Member

@sansyrox sansyrox left a comment

Choose a reason for hiding this comment

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

Thanks for the great work @carlosm27 😄
I have one comment

sansyrox
sansyrox previously approved these changes Apr 6, 2023
Copy link
Member

@sansyrox sansyrox left a comment

Choose a reason for hiding this comment

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

@carlosm27 , thanks for the updated PR but there is still one merge conflict. We will be able to hopefully merge it after that 😅

@sansyrox sansyrox dismissed their stale review April 6, 2023 08:32

approved by mistake

Copy link
Member

@sansyrox sansyrox left a comment

Choose a reason for hiding this comment

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

Great work @carlosm27 🔥

LGTM!

@sansyrox sansyrox merged commit 241d7fa into sparckles:main Apr 7, 2023
@carlosm27
Copy link
Contributor Author

Great work @carlosm27 🔥

LGTM!

Thank you @sansyrox for guiding me and helping me through the process.

@carlosm27 carlosm27 deleted the path_params branch April 22, 2023 00:46
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.

Proposal to rename params with path_params
2 participants