Skip to content

Conversation

@DePasqualeOrg
Copy link
Contributor

@DePasqualeOrg DePasqualeOrg commented Dec 2, 2025

It's safer to use appending(path:) or appending(component:) instead of string interpolation to construct URLs. The latter properly encodes the slash in revisions like pr/1, which would otherwise result in a 404 error.

Before: pr/1 → two path segments → 404
After: pr/1pr%2F1 → works

I've also replaced the use of the deprecated appendingPathComponent in the same file with appending(path:).

@DePasqualeOrg DePasqualeOrg force-pushed the fix-hub-api-url-construction branch from fde6255 to 3f6ec25 Compare December 2, 2025 09:36
@DePasqualeOrg DePasqualeOrg marked this pull request as draft December 2, 2025 09:41
@DePasqualeOrg DePasqualeOrg marked this pull request as ready for review December 2, 2025 09:42
@DePasqualeOrg DePasqualeOrg marked this pull request as draft December 2, 2025 11:32
@DePasqualeOrg DePasqualeOrg force-pushed the fix-hub-api-url-construction branch 2 times, most recently from 76d1387 to 10209d9 Compare December 2, 2025 11:53
@DePasqualeOrg DePasqualeOrg marked this pull request as ready for review December 2, 2025 11:59
@DePasqualeOrg DePasqualeOrg marked this pull request as draft December 2, 2025 11:59
@DePasqualeOrg DePasqualeOrg force-pushed the fix-hub-api-url-construction branch from 10209d9 to 96333f1 Compare December 2, 2025 12:06
@DePasqualeOrg DePasqualeOrg marked this pull request as ready for review December 2, 2025 12:15
Copy link
Collaborator

@mattt mattt left a comment

Choose a reason for hiding this comment

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

This is a good set of changes. Thanks for opening this PR, @DePasqualeOrg 🫶

Copy link
Member

@pcuenca pcuenca left a comment

Choose a reason for hiding this comment

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

Makes sense, thanks a lot @DePasqualeOrg, and sorry for the oversight with pr/1!

@pcuenca pcuenca merged commit fae7768 into huggingface:main Dec 4, 2025
2 checks passed
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.

3 participants