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 Pedestal Issue #610 #746

Merged
merged 2 commits into from
May 12, 2023
Merged

Fix Pedestal Issue #610 #746

merged 2 commits into from
May 12, 2023

Conversation

bonkydog
Copy link
Contributor

#610

url-for throws and exception when a context path is supplied and used for a route with a path parameter in the first segment.

The logic appeared to assume that path segments are always strings and was using empty? to check for the empty string. empty? expects a sequence. A keyword is not a sequence, so empty was throwing an exception.

url-for throws and exception when a context path is supplied and used for a route with a path parameter in the first segment.
@bonkydog bonkydog requested a review from hlship May 10, 2023 20:43
Copy link
Contributor

@hlship hlship left a comment

Choose a reason for hiding this comment

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

Maybe delete that comment before merging.

@@ -249,7 +249,7 @@
:context-path-parts context-path-parts)
;;(concat context-path-parts path-parts)
Copy link
Contributor

Choose a reason for hiding this comment

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

This comment should be deleted.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ah, thanks!

@bonkydog bonkydog merged commit 6849dc1 into master May 12, 2023
1 check passed
@bonkydog bonkydog deleted the issue-610 branch May 12, 2023 16:39
hlship added a commit that referenced this pull request Dec 21, 2023
@hlship hlship added the bug label Dec 21, 2023
@hlship hlship modified the milestone: 0.7.0 Dec 21, 2023
@hlship
Copy link
Contributor

hlship commented Dec 21, 2023

Above changes accidental, this was fixed in 0.6.0.

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

Successfully merging this pull request may close these issues.

None yet

2 participants