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

About function names #195

Open
WurmPeter opened this issue Aug 13, 2024 · 2 comments
Open

About function names #195

WurmPeter opened this issue Aug 13, 2024 · 2 comments
Labels
help wanted Extra attention is needed

Comments

@WurmPeter
Copy link
Contributor

What always confuses me: there are functions beginning with "log" that don't cause a logging action, but somehow change the logging behaviour.

Functions that actually log:

- log_level()
- log_fatal()
- log_error()
- log_warn()
- log_success()
- log_info()
- log_debug()
- log_trace()
- log_eval()
- log_failure()
- log_tictoc()
- log_separator()
- log_with_separator()

Functions that don't log, but change logger's behaviour:

- log_threshold()
- log_appender()
- log_formatter()
- log_layout()
- log_messages()
- log_warnings()
- log_errors()
- logger()
- delete_logger_index()

Helpers:

- with_log_threshold()
- log_shiny_input_changes()
- get_logger_meta_variables()
- log_namespaces()
- log_indices()
- ...

I know it's a bit late to change anything and breaks the API, but out of curiosity: Wouldn't the function names be more understandable if those that change logging behaviour would be prefixed with "logger_"? So

- logger()
- logger_threshold()
- logger_appender()
- logger_formatter()
- logger_layout()
- logger_messages()
- logger_warnings()
- logger_errors()
- logger_delete_index()
- logger_get_meta_variables()
- logger_get_namespaces()
- logger_get_indices()

Or is it just me?

@daroczig
Copy link
Owner

Hmm, good question. I don't feel super comfortable about the suggested naming schema, but I understand that the old design might be confusing. Let's collect feedback here from the community and take it from there.

@daroczig daroczig added the help wanted Extra attention is needed label Aug 17, 2024
@thomasp85
Copy link
Contributor

I tend to agree that the current situation is confusing. FWIW we have at time taken the plunge and renamed poorly named functions in tidyverse using a very lenient deprecation period. My suggestion would be to have clearly market getter and setter functions for global state

- logger_set_threshold()
- logger_set_appender()
- logger_set_formatter()
- logger_set_layout()
- logger_get_meta_variables()
- logger_get_namespaces()
- logger_get_indices()
- logger_delete_index()

I would also go for logger_create() rather than logger() as it is a bit unclear from the name what it does

lastly I could see a family of hook functions:

- logger_hook_errors()
- logger_hook_warnings()
- logger_hook_messages()
- logger_hook_shiny_input()

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

3 participants