mesos-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Ben Mahler" <benjamin.mah...@gmail.com>
Subject Re: Review Request 20850: Customize the configuration of logging level
Date Wed, 07 May 2014 23:31:17 GMT

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20850/#review42438
-----------------------------------------------------------

Ship it!


Looks great Alexandra! I'll get this committed! There was a minor compile issue and some style
issues that I'll get cleaned up for you.


src/logging/logging.cpp
<https://reviews.apache.org/r/20850/#comment76242>

    What are the implications of -1 here? A comment would be great, I'll update this to be
INFO by default with a TODO.



src/logging/logging.cpp
<https://reviews.apache.org/r/20850/#comment76241>

    Why was this moved down? Shouldn't we be validating input prior to creating a directory?
    
    I'll move this up but please let me know if there was a reason to move it down!



src/logging/logging.cpp
<https://reviews.apache.org/r/20850/#comment76240>

    Hm.. looks like this is missing a closing '"' character?
    
    Let's align the newlines on the " character as well.



src/logging/logging.cpp
<https://reviews.apache.org/r/20850/#comment76239>

    How about s/severityName/severity/ ? How about just doing it inline?



src/slave/slave.cpp
<https://reviews.apache.org/r/20850/#comment76238>

    How about: s/logging_level/severity/ ?
    
    Could we just call it inline?
    
    Try<string> log = logging::getLogFile(
        logging::getLogSeverity(flags.logging_level);


- Ben Mahler


On May 1, 2014, 11:09 a.m., Alexandra Sava wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20850/
> -----------------------------------------------------------
> 
> (Updated May 1, 2014, 11:09 a.m.)
> 
> 
> Review request for mesos.
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> Rename 'minloglevel' flag in 'logging_level' in order to be consistent with the existing
flags.
> 
> Rename 'getMinLogLevel' function in 'getLogSeverity' to be more suggestive for what it
does.                                                          
> 
> Disallow configuring FATAL logging because it's a special case and it involves too many
changes across the entire source code.                                 
> 
> Create validation for the 'logging_level' flag in order to notify the user if it had
entered an unsupported value.                                         
> 
> Update the documentation for 'logging_level' flag, with the values it can take.
> 
> 
> Diffs
> -----
> 
>   src/logging/flags.hpp 457feeeb7ca7ececf3c4e69189800ecb370053e6 
>   src/logging/logging.hpp 39c2934f733e5c058f62b5497f31f7a7057bb4c7 
>   src/logging/logging.cpp 176e49a6a2aef13be44ff910144c7321c942345a 
>   src/master/master.cpp f205dca43f10697862e3fd3f435f1127a9d0aecb 
>   src/slave/slave.cpp cb80609ba421b3b9a4664e600f0e53ecab8574c4 
>   src/webui/master/static/js/controllers.js 4b8487e0c285f892ad352993c81637f38df1429f

> 
> Diff: https://reviews.apache.org/r/20850/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Alexandra Sava
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message