Skip to content

Update normalize_path like Rails #2536

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
Merged
merged 5 commits into from
Feb 19, 2025
Merged

Update normalize_path like Rails #2536

merged 5 commits into from
Feb 19, 2025

Conversation

@ericproulx
Copy link
Contributor

This PR apply the same strategy as Rails when normalizing path.
@ericproulx ericproulx marked this pull request as ready for review February 16, 2025 13:18
@ericproulx ericproulx requested a review from dblock February 16, 2025 13:18
@grape-bot
Copy link

grape-bot commented Feb 16, 2025 •
edited
Loading

1 Warning
⚠️ There're library changes, but not tests. That's OK as long as you're refactoring existing code.

Generated by 🚫 Danger

dblock
dblock requested changes Feb 18, 2025
View reviewed changes
Copy link
Member

@dblock dblock left a comment •
edited
Loading

Choose a reason for hiding this comment

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

In what situations does this change behavior? There's a lot more going on here than in the Rails commit :) At the very least I feel like we need to bring in some tests from Rails for normalize_path if we're going to be doing it here as well.
@ericproulx ericproulx requested a review from dblock February 19, 2025 12:16
@ericproulx
Copy link
Contributor Author

@dblock I've added some specs. Basically, it does exactly the same thing but there's a faster path for the usual cases.
@dblock dblock merged commit 6e6958f into ruby-grape:master Feb 19, 2025
63 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

3 participants

@ericproulx @grape-bot @dblock