> On Sept. 14, 2012, 4:46 p.m., Benjamin Hindman wrote:
> > src/logging/logging.hpp, lines 38-40
> > <https://reviews.apache.org/r/7061/diff/3/?file=153043#file153043line38>
> >
> >     I wonder if the right thing to do here is actually just expose argv0 
> > somewhere (maybe not logging specific) and then do 'os::basename(argv0) + 
> > suffix' when you want to get the log name. Note as well that "basename" 
> > typically refers to the right most suffix, so basename(/foo/bar/baz.txt) => 
> > baz.txt and basename(baz.txt) => txt.

1. So I've changed the name to mimic the missing function in glog: 
getLogDestination(), let me know if you prefer another name.

2. It seems a bit weird to me for non-logging code to be aware of how to 
compute the log destination. I think that logic should be in one location, 
rather thank sprinkled throughout the code, do you agree? Say we upgraded glog 
and they fix the issue: 
http://code.google.com/p/google-glog/issues/detail?id=116 , then we could kill 
this function easily :)


- Ben


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


On Sept. 14, 2012, 6:20 p.m., Ben Mahler wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/7061/
> -----------------------------------------------------------
> 
> (Updated Sept. 14, 2012, 6:20 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Vinod Kone.
> 
> 
> Description
> -------
> 
> This eliminates the hard-coding of the log filenames: mesos-master.INFO and 
> mesos-slave.INFO.
> 
> 
> Diffs
> -----
> 
>   src/logging/logging.hpp 9dcc7c1 
>   src/logging/logging.cpp d6d31ec 
>   src/master/master.cpp addabdb 
>   src/slave/slave.cpp 4ea1db1 
>   src/webui/master/static/controllers.js 41cec18 
> 
> Diff: https://reviews.apache.org/r/7061/diff/
> 
> 
> Testing
> -------
> 
> make check
> mesos-local.sh runs
> 
> 
> Thanks,
> 
> Ben Mahler
> 
>

Reply via email to