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

Add Short GUID support (#3) #4

Closed
wants to merge 5 commits into from
Closed

Add Short GUID support (#3) #4

wants to merge 5 commits into from

Conversation

danieljsummers
Copy link

There are three different commits here (initial PR). I did them this way because I did 3 different things:

  • The first updated the dependencies against the latest Giraffe (it was still targeting 2.0.x) and fixed the unit tests that change broke (Giraffe now put the character set as part of the content type).
  • The second adds in the short GUID support, by simply calling out to Giraffe's implementation. It's one if statement on the GUID parsing logic, so hopefully that won't slow things down. I added a test for this third GUID format.
  • The third adds the HEAD and OPTIONS route groupings; this will provide a workaround for TokenRouter needs explicit GET when POST is used #1 (oops - think I said number 2 on the commit) in cases where the only reason they aren't under a verb is that one of those needs to be mapped. (This is an actual use case I have.) I also added a test for each verb to make sure that they worked as advertised.

At any rate, feel free to squash and/or cherry-pick as you see fit. Also, if something needs to be done differently, just let me know.

This was still targeting a 2.0 release of Giraffe; this changes it to 3.x, and fixes tests that broke as part of the upgrade (content type now has charset as well)
This utilizes `ShortGuid.toGuid` on a 22-character `%O` input
This provides a workaround for #2 - if the route is outside a verb because the user needs these mapped, this lets them explicitly map them
Looks like AppVeyor doesn't have 104 yet...
@danieljsummers
Copy link
Author

I'm out of time for this evening. The dotnet version is what's throwing these builds off. Giraffe 3.4 is the last version to target 2.1; let me know if you want me to rework the deps and target the latest available on the dotnet SDK 2.1 series.

@danieljsummers danieljsummers closed this by deleting the head repository Apr 3, 2024
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.

1 participant