diff mbox series

log: add l_notice

Message ID 20240213131619.828613-1-prestwoj@gmail.com (mailing list archive)
State Accepted, archived
Headers show
Series log: add l_notice | expand

Checks

Context Check Description
tedd_an/pre-ci_am success Success
prestwoj/iwd-ci-build success Build - Configure
prestwoj/iwd-ci-makecheckvalgrind success Make Check w/Valgrind
prestwoj/iwd-ci-clang success clang PASS
prestwoj/iwd-ci-makecheck success Make Check
prestwoj/iwd-ci-makedistcheck success Make Distcheck
prestwoj/iwd-ci-testrunner success test-runner PASS

Commit Message

James Prestwood Feb. 13, 2024, 1:16 p.m. UTC
Adds the ability to use a notice (5) log level.
---
 ell/log.h | 2 ++
 1 file changed, 2 insertions(+)

Comments

Marcel Holtmann Feb. 13, 2024, 2:05 p.m. UTC | #1
Hi James,

> Adds the ability to use a notice (5) log level.
> ---
> ell/log.h | 2 ++
> 1 file changed, 2 insertions(+)
> 
> diff --git a/ell/log.h b/ell/log.h
> index 72dbed3..7ddf663 100644
> --- a/ell/log.h
> +++ b/ell/log.h
> @@ -17,6 +17,7 @@ extern "C" {
> 
> #define L_LOG_ERR 3
> #define L_LOG_WARNING 4
> +#define L_LOG_NOTICE 5
> #define L_LOG_INFO 6
> #define L_LOG_DEBUG 7

for what? What is the difference to just use l_info?

Regards

Marcel
Denis Kenzior Feb. 14, 2024, 4:18 p.m. UTC | #2
Hi Marcel,

>> #define L_LOG_WARNING 4
>> +#define L_LOG_NOTICE 5
>> #define L_LOG_INFO 6
>> #define L_LOG_DEBUG 7
> 
> for what? What is the difference to just use l_info?

Per conversation on Slack, James wants to categorize certain messages for easier 
searching.  Since NOTICE level exists, I suggested it would fit the usecase.

Regards,
-Denis
Marcel Holtmann Feb. 14, 2024, 4:27 p.m. UTC | #3
Hi Denis,

>>> #define L_LOG_WARNING 4
>>> +#define L_LOG_NOTICE 5
>>> #define L_LOG_INFO 6
>>> #define L_LOG_DEBUG 7
>> for what? What is the difference to just use l_info?
> 
> Per conversation on Slack, James wants to categorize certain messages for easier searching.  Since NOTICE level exists, I suggested it would fit the usecase.

I never really liked having many different levels. That was the reason I left l_notice out initially. I see no real benefit of it except us discussing "is this suppose to be l_info and l_notice" and in the end it will be just random.

So if you tell me / convince me when to use l_notice of l_info or use l_warn instead of l_notice, then we can talk.

Regards

Marcel
Denis Kenzior Feb. 14, 2024, 4:40 p.m. UTC | #4
Hi Marcel,

On 2/14/24 10:27, Marcel Holtmann wrote:
> Hi Denis,
> 
>>>> #define L_LOG_WARNING 4
>>>> +#define L_LOG_NOTICE 5
>>>> #define L_LOG_INFO 6
>>>> #define L_LOG_DEBUG 7
>>> for what? What is the difference to just use l_info?
>>
>> Per conversation on Slack, James wants to categorize certain messages for easier searching.  Since NOTICE level exists, I suggested it would fit the usecase.
> 
> I never really liked having many different levels. That was the reason I left l_notice out initially. I see no real benefit of it except us discussing "is this suppose to be l_info and l_notice" and in the end it will be just random.

There will always be some of that.  Each project will have to calibrate 
individually, but I don't see a problem exposing this inside ell.

In iwd we use l_info for purely informational messages, like hardware 
capabilities.  Most users won't care.  l_notice would be used for things like 
roaming decision logic and other messages that do not warrant a l_warn / 
l_error.  I think it is a nice in-between actually, particularly on non-user 
facing systems.

Regards,
-Denis
Marcel Holtmann Feb. 14, 2024, 4:54 p.m. UTC | #5
Hi Denis,

>>>>> #define L_LOG_WARNING 4
>>>>> +#define L_LOG_NOTICE 5
>>>>> #define L_LOG_INFO 6
>>>>> #define L_LOG_DEBUG 7
>>>> for what? What is the difference to just use l_info?
>>> 
>>> Per conversation on Slack, James wants to categorize certain messages for easier searching.  Since NOTICE level exists, I suggested it would fit the usecase.
>> I never really liked having many different levels. That was the reason I left l_notice out initially. I see no real benefit of it except us discussing "is this suppose to be l_info and l_notice" and in the end it will be just random.
> 
> There will always be some of that.  Each project will have to calibrate individually, but I don't see a problem exposing this inside ell.
> 
> In iwd we use l_info for purely informational messages, like hardware capabilities.  Most users won't care.  l_notice would be used for things like roaming decision logic and other messages that do not warrant a l_warn / l_error.  I think it is a nice in-between actually, particularly on non-user facing systems.

fine, then lets get a section into the coding style document.

Regards

Marcel
Denis Kenzior Feb. 14, 2024, 4:57 p.m. UTC | #6
James,

> 
> fine, then lets get a section into the coding style document.

Can you come up with something?

I've applied this patch, thanks.

Regards,
-Denis
James Prestwood Feb. 14, 2024, 6:28 p.m. UTC | #7
On 2/14/24 8:57 AM, Denis Kenzior wrote:
> James,
>
>>
>> fine, then lets get a section into the coding style document.
>
> Can you come up with something?

Yep sure, but are you talking about IWD's or ELL's coding style 
document? l_log isn't used in ELL so I'm not sure its relevant there, 
and any external project using l_log could really do whatever they want 
with it.

For IWD yes, I planned on adding a coding style section along with my 
patches using l_notice as its going to be reserved for very specific 
use-cases and have some basic syntax requirements for tooling to parse.

Thanks,

James

>
> I've applied this patch, thanks.
>
> Regards,
> -Denis
Denis Kenzior Feb. 14, 2024, 6:51 p.m. UTC | #8
Hi James,

On 2/14/24 12:28, James Prestwood wrote:
> 
> On 2/14/24 8:57 AM, Denis Kenzior wrote:
>> James,
>>
>>>
>>> fine, then lets get a section into the coding style document.
>>
>> Can you come up with something?
> 
> Yep sure, but are you talking about IWD's or ELL's coding style document? l_log 

iwd

> isn't used in ELL so I'm not sure its relevant there, and any external project 
> using l_log could really do whatever they want with it.
> 
> For IWD yes, I planned on adding a coding style section along with my patches 
> using l_notice as its going to be reserved for very specific use-cases and have 
> some basic syntax requirements for tooling to parse.

Awesome.  Sounds good.

Regards,
-Denis
diff mbox series

Patch

diff --git a/ell/log.h b/ell/log.h
index 72dbed3..7ddf663 100644
--- a/ell/log.h
+++ b/ell/log.h
@@ -17,6 +17,7 @@  extern "C" {
 
 #define L_LOG_ERR	3
 #define L_LOG_WARNING	4
+#define L_LOG_NOTICE	5
 #define L_LOG_INFO	6
 #define L_LOG_DEBUG	7
 
@@ -84,6 +85,7 @@  void l_debug_disable(void);
 
 #define l_error(format, ...)  l_log(L_LOG_ERR, format, ##__VA_ARGS__)
 #define l_warn(format, ...)   l_log(L_LOG_WARNING, format, ##__VA_ARGS__)
+#define l_notice(format, ...) l_log(L_LOG_NOTICE, format, ##__VA_ARGS__)
 #define l_info(format, ...)   l_log(L_LOG_INFO, format, ##__VA_ARGS__)
 #define l_debug(format, ...)  L_DEBUG_SYMBOL(__debug_desc, format, ##__VA_ARGS__)