-
Notifications
You must be signed in to change notification settings - Fork 12
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
Delete HttpResponseStatusCodes.java and statusCode field #199
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The reason the methods in this file returns itself is because its a fluent api. So we on a Configuration object can write like this:
config.setPort(8080).setWebRoot("/path");
Therefor I don't think it's a good idea to change the return type. We should also avoid changing the signature and return value from public methods because it will affect everyone using the methods.
Qodana Community for JVMIt seems all right 👌 No new problems were found according to the checks applied 💡 Qodana analysis was run in the pull request mode: only the changed files were checked View the detailed Qodana reportTo be able to view the detailed Qodana report, you can either:
To get - name: 'Qodana Scan'
uses: JetBrains/qodana-action@v2023.3.1
with:
upload-result: true Contact Qodana teamContact us at qodana-support@jetbrains.com
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lets try that. Approved from me.
@fungover/java23 We need another review on this PR. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems to be good!
In my opnion we should remove this class because the only reason we had it in the first place was because of the string.