fix: make `sorted_routes` perform natural sort issues/22

alexschimpf / fastapi-versionizer

FastAPI Versionizer

MIT License

81 stars 13 forks source link

fix: make `sorted_routes` perform natural sort issues/22 #23

Closed ndejong closed 1 year ago

ndejong commented 1 year ago

Pull Request Checklist

[x] Make sure to read the contributor guide here.
[ ] Make sure you are making a pull request against the staging branch.
[x] Make sure your pull request title matches the requested format.
[x] Make sure to lint, type-check, and run unit and API tests.

Description

Fix for https://github.com/alexschimpf/fastapi-versionizer/issues/22

alexschimpf commented 1 year ago

But what about a case like this?

/v10.3/something
/v1.5/something
/v1.10/something
/v2.0/something

You'll end up with an ordering like:

['/v1.10/something', '/v1.5/something', '/v10.3/something', '/v2.0/something']

Do we want to handle this kind of thing as well? There is a lib here that handles natural sorting for you, including handling whitespace as well as numbers.

alexschimpf commented 1 year ago

Also, the pre-commit hook should handle making sure you make your commit messages are written in a standard way. Looks like your commits are failing that check currently on this PR.

If it's easier, I can just squash your commits on merge for you, so it's no issue. But please keep it in mind.

ndejong commented 1 year ago

Urgh - yes, I was trying to solve a problem I had with route paths without the need for a new dependency, totally neglected semantic sort situations :(

Some more discussion needed, I'll move that to https://github.com/alexschimpf/fastapi-versionizer/issues/22 to save cluttering up this PR.

Also - on the commit message normalization, I did see this and thought I'd resolved it with a (messy on my behalf) update on the git commit message - that failed - I'll address on the next

alexschimpf commented 1 year ago

Closing this PR as it's been taken care of already