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 HttpStatus enum for status code #263

Closed
wants to merge 1 commit into from

Conversation

rbygrave
Copy link
Contributor

@rbygrave rbygrave commented Aug 7, 2023

Not ideal in the sense that it has the __not_set(0) entry. The default status codes depend on the HTTP method + if the return body is empty (void methods).

An alternative is to have a separate annotation like @Status or @ResponseStatus and remove the statusCode attributes from @Produces and @ExceptionHandler (and then the __not_set entry could be remove from the HttpStatus enum).

Also not ideal in that this PR just copies the enum for the generator.

@@ -0,0 +1,84 @@
package io.avaje.http.api;

public enum HttpStatus {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

idk, most microframeworks provide their own HTTP status enum

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But how would they use it on an avaje annotation?

Currently we use an int meaning folks need to know the http status codes which is maybe OK but in theory this helps them. Is there a better way?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Idk, I like using int

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, we can stick with int.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have the thought of adding a java comment in the generated code [where the generated code sets the status] that translates the status code int into the reason description (for the purpose of helping the person reading the generated code who may not know the status code being used).

That way we use int, but at least the generated code might be more readable for devs reading the [generated] code later.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sounds good to me

@rob-bygrave rob-bygrave closed this Aug 8, 2023
@rbygrave rbygrave deleted the feature/rename-defaultStatys branch October 3, 2023 09:24
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.

3 participants