nerdishbynature / octokit.swift

A Swift API Client for GitHub and GitHub Enterprise
MIT License
488 stars 126 forks source link

Async notifications support + added endpoints #183

Open LucasCoderT opened 4 months ago

LucasCoderT commented 4 months ago
LucasCoderT commented 4 months ago

Before merging, I've found an issue.

For the pagination params around notifications, if I keep them as String. The API returns wrong/old/invalid data. But if I change them to be Int Then the router fails to encode them https://github.com/nerdishbynature/RequestKit/blob/main/Sources/RequestKit/Router.swift#L127

But then the API returns correct data despite this. so I'm not sure what exactly is going on here.

pietbrauer commented 4 months ago

@LucasCoderT Have you looked at the resulting URLs? Are they the same? URL Params don't have types so I doubt page=2 (encoded as Int) makes a difference to page=2 encoded as String?

LucasCoderT commented 4 months ago

@pietbrauer

Apologies, I think this had something to do with my local environment. When I tried it on my laptop instead of my desktop it responds and follows the params as expected. Weird. This PR should be good to go then 👍