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

Update Lucky to use new HTTP::Status #747

Closed
jwoertink opened this issue Apr 5, 2019 · 2 comments · Fixed by #769
Closed

Update Lucky to use new HTTP::Status #747

jwoertink opened this issue Apr 5, 2019 · 2 comments · Fixed by #769

Comments

@jwoertink
Copy link
Member

Crystal just had this commit which adds in an HTTP::Status enum. Lucky has this defined, but it's missing my favorite status 418. I think we should just use the built in crystal one after crystal 0.28.0 is released.

The cool thing too, is the next version of crystal will give us compile time deprecation notices. I haven't tried this, but I think we could do something like:

@[Deprecated("Lucky::Action::Status deprecated, please use HTTP::Status instead")]
alias Status = HTTP::Status
@jwoertink
Copy link
Member Author

Ok, so this one might be a little tricky. It's not as simple as just doing an alias because of the casing, and crystal enums treat different casing as different values.

Also, apparently the @[Deprecated] annotation only works on methods. As far as I know, there's not a way to log out a deprecation warning when calling individual enums.

So my suggestion is to hard flip these over. It would be a complete breaking change; however, we changed the mime type thing that was also a breaking change anyway.

@paulcsmith
Copy link
Member

@jwoertink I think a full breaking change is a-ok. We'll do it in a new release so people can upgrade when they are ready 👍

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 a pull request may close this issue.

2 participants