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 badges option to @tinyhttp/logger #51

Closed
talentlessguy opened this issue Aug 18, 2020 · 13 comments · Fixed by #55
Closed

Add badges option to @tinyhttp/logger #51

talentlessguy opened this issue Aug 18, 2020 · 13 comments · Fixed by #55
Labels
enhancement New feature or request good first issue Good for newcomers logger Affects `@tinyhttp/logger` middleware Affects middleware packages

Comments

@talentlessguy
Copy link
Member

talentlessguy commented Aug 18, 2020

Is your feature request related to a problem? Please describe.

It adds emoji badges for better readability

Describe the solution you'd like

Add badges setting with 2 sub-settings to be used like this:

app.use(logger({ badges: { emoji: true, captions: false }))

Default values are both false - if badges: true - only toggle emoji. if an object is present - check for the settings.

first setting toggles emoji badges, like this ⚠️
second enables emoji alt text (in case emoji fails to display or just prefer the text instead), like this: warning

here's a table of emoji / texts:

emoji text status code
Error 500
⚠️ status message 4XX (Except 404)
🆗 OK 2XX
🔎 Not Found 404

Should be logged in the beginning:

🆗 GET 200 OK /css/shared.css

Additional context

Zoya logger that has emoji:

image

@talentlessguy talentlessguy added enhancement New feature or request good first issue Good for newcomers middleware Affects middleware packages logger Affects `@tinyhttp/logger` labels Aug 18, 2020
@ahmad-reza619
Copy link
Contributor

ahmad-reza619 commented Aug 27, 2020

Hi there @talentlessguy , i would like to try these on, is this mean to create a new type of middleware? or just extending logger middleware?

@talentlessguy
Copy link
Member Author

talentlessguy commented Aug 27, 2020

@ahmad-reza619 hello, nope, as labeled on the issue, it should be done inside @tinyhttp/logger

I have a mistake in the original code that leads to confusion, my bad

edit: fixed

@ahmad-reza619
Copy link
Contributor

I'm sorry for not noticing the label 😅

@talentlessguy
Copy link
Member Author

@ahmad-reza619 it's fine, you can make a PR at any time you want, and the setup instructions for this repo are in CONTRIBUTING.md file

let me know if you have any questions and don't be afraid to ask them here

@ahmad-reza619
Copy link
Contributor

@talentlessguy what about 3XX status code?

@ahmad-reza619
Copy link
Contributor

ahmad-reza619 commented Aug 28, 2020

and also, for captions what should be logged? the status or emoji name?
because you were saying

second enables emoji alt text (in case emoji fails to display or just prefer the text instead), like this: warning

but the tables didn't have any text like warning

@talentlessguy
Copy link
Member Author

@talentlessguy what about 3XX status code?

not sure which emoji to pick, will write back here latter

@talentlessguy
Copy link
Member Author

and also, for captions what should be logged? the status or emoji name?
because you were saying

second enables emoji alt text (in case emoji fails to display or just prefer the text instead), like this: warning

but the tables didn't have any text like warning

I meant alt text for emoji symbols, it can be found on Emojipedia

@talentlessguy
Copy link
Member Author

@ahmad-reza619 pardon for changing requiremenets, I think instead of coming up with our own emojis (and having to think which ones suit better) we better use this module:

https://github.com/bendrucker/http-status-emojis/blob/master/index.js

@ahmad-reza619
Copy link
Contributor

@talentlessguy what about the alt text?

@talentlessguy
Copy link
Member Author

@talentlessguy tbh I'm not sure about that we need it now... maybe it's useless?

I mean something like

500 bomb GET /

doesn't make much sense

probably not worth adding it, just the emoji then

@talentlessguy
Copy link
Member Author

@all-contributors please add ahmad-reza619 for code

@allcontributors
Copy link
Contributor

@talentlessguy

I've put up a pull request to add @ahmad! 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers logger Affects `@tinyhttp/logger` middleware Affects middleware packages
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants