You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Use pathSegment(_:) for the remaining inline path encodings #112
Noted while doing #95 (see PR #111), which collapsed the query-value encoders. The path side has the
same shape and was deliberately left alone to keep that PR a single concern.
These are correct as written. Unlike the query-value case, there is no latent bug here — .urlPathAllowed is the right set for a path segment, and the inline form produces exactly what pathSegment(_:) would. This is consistency and discoverability, not correctness.
The reason it is worth doing at all: with two near-identical idioms in the file, the next person
copying one has a coin-flip chance of reaching for the wrong one, and picking .urlQueryAllowed for
a query value is precisely the bug #95 just spent a PR removing. #111 put queryValue(_:) directly
beside pathSegment(_:) with a comment saying they are not interchangeable; converting the
stragglers makes the file self-consistent so the comment is reinforced rather than contradicted.
Build this
Replace inline .urlPathAllowed encodings of a path segment with pathSegment(_:).
Leave anything that is not a single path segment alone — if a call site is encoding a whole
path or something with intentional slashes, pathSegment(_:) is the wrong tool and it should
keep doing what it does, with a comment saying why.
No behaviour change. If any conversion changes a single byte on the wire, stop and say so —
that would mean the two forms were not equivalent after all.
Acceptance criteria
No inline .urlPathAllowed path-segment encoding remains, or each survivor has a one-line
comment explaining why it is not using the helper.
Suite green with no test changes needed — a pure refactor should not require them.
Files:Services/APIClient.swift and the APIClient+*.swift extensions.
I wrote "roughly a dozen call sites", carrying over the estimate from #111's report. Counted
properly on main:
File
Inline .urlPathAllowed encodings
APIClient.swift
92
ILWebURL.swift
3
APIClient+Documents.swift
1
APIClient+Organizations.swift
1
APIClient+ListContributors.swift
1
APIClient+GitHub.swift
1
APIClient+Lists.swift
1
APIClientTransport.swift
1 (the helper's own definition — leave it)
So ~100, not ~12. That changes the character of the task: it is a large mechanical sweep rather than
a quick tidy, and APIClient.swift alone will produce a diff that conflicts with anything else in
flight in that file.
Sequencing suggestion: run it when nothing else is touching APIClient.swift, and ideally as one
commit that is purely mechanical, so a reviewer can scan it as find-and-replace rather than reading
92 individual changes.
ILWebURL.swift is worth a second look rather than blind conversion — it builds web URLs rather
than API request paths, so it may not have access to the transport helper at all, and its three sites
may be correct exactly as they are.
Activity
Adron commentedon Sep 17, 2026
Correcting the size estimate in the description
I wrote "roughly a dozen call sites", carrying over the estimate from #111's report. Counted
properly on
main:.urlPathAllowedencodingsAPIClient.swiftILWebURL.swiftAPIClient+Documents.swiftAPIClient+Organizations.swiftAPIClient+ListContributors.swiftAPIClient+GitHub.swiftAPIClient+Lists.swiftAPIClientTransport.swiftSo ~100, not ~12. That changes the character of the task: it is a large mechanical sweep rather than
a quick tidy, and
APIClient.swiftalone will produce a diff that conflicts with anything else inflight in that file.
Sequencing suggestion: run it when nothing else is touching
APIClient.swift, and ideally as onecommit that is purely mechanical, so a reviewer can scan it as find-and-replace rather than reading
92 individual changes.
ILWebURL.swiftis worth a second look rather than blind conversion — it builds web URLs ratherthan API request paths, so it may not have access to the transport helper at all, and its three sites
may be correct exactly as they are.