-
Notifications
You must be signed in to change notification settings - Fork 7
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
Logger Comment Formatting #50
Comments
This comment was marked as outdated.
This comment was marked as outdated.
/start |
! This task does not reflect a business priority at the moment. You may start tasks with one of the following labels: Priority: 3 (High), Priority: 4 (Urgent), Priority: 5 (Emergency) |
I think those don't make sense for a logger, Currently we have:
I think these are fine, although ok() and info() are rarely used, we could rename ok() to success() because I think Note logger.success() and logger.debug() but debug usually isn't posted Tip I think this one doesn't fit any of our methods Important Not sure about this one either Warning logger.error() Caution logger.fatal() |
The functions basically match the |
i feel like we should either make this note/tip/important/warning/caution as SDK functions separate from the logger or match them with logger methods, or both. |
Small details but I think the general spirit of this proposal still holds. The idea is to guide the developer to think how http codes are standardized. It makes the log type less subjective to choose. In practice we use "redirect" as developer verbose, purple, comments.
I disagree. Client error means the user messed up. Server error means that the code is messed up. Examples:
recreating the visualsI agree that the headers can be misleading. We can possibly recreate them without the headers by using images of the colored borders, but I'm not sure if we can float: left images in GitHub comments. |
If client error will appear as Warning and server error will appear as Caution, then the user won't differentiate between those two. Even if you add status codes only developers will understand that. It's the text that will the user that they used the command with invalid parameters. If we could modify the header that would make more sense |
The errors just need to be descriptive and it's fine. |
/start |
! This task does not reflect a business priority at the moment. You may start tasks with one of the following labels: Priority: 3 (High), Priority: 4 (Urgent), Priority: 5 (Emergency) |
@whilefoo the deadline is at Wed, Dec 18, 2:01 AM UTC |
@whilefoo can you handle this |
Important
|
Passed the disqualification threshold and no activity is detected, removing assignees: @whilefoo. |
@whilefoo please handle this promptly |
Change all logger comments to "new format" which looks more aesthetically pleasing.
Old Format
We should color code essentially the equivalent of HTTP 1xx 2xx 3xx 4xx 5xx class errors in these bot comments.
# 1xx Class (Informational Responses)
+ 2xx Class (Successful Responses)
@@ 3xx Class (Redirection Messages) @@
! 4xx Class (Client Error Responses)
- 5xx Class (Server Error Responses)
New Format
As it turns out, GitHub flavored markdown includes these handy color indicators. We can use the same color keys but instead of diff syntax, we use this syntax.
This should be applied across all kernel and plugin messages.
It might even be handy to rename the methods in our logger if its still attached to posting comments. The inspiration behind the rename is so that the class of message is no longer subjective.
Status Codes
Based on HTTP status codes, we should color code the responses. The only problem is that some of the headers can be misleading, but it does look a lot more aesthetically pleasing than the diffs. The left border in particular I think is a great minimal representation of the status.
Note
1xx
Tip
2xx
Important
3xx
Warning
4xx
Caution
5xx
Source Code
About HTTP Status Code Classes
The text was updated successfully, but these errors were encountered: