Conversation
|
is there any difference between those two files? |
| local app_name="$1" | ||
| local datetime=`date +%Y/%m/%d/%H` | ||
| local app_log_dir="{{ app_log_dir }}" | ||
| local logfile="${app_log_dir}/${datetime}/${app_name}.log" |
There was a problem hiding this comment.
So it looks like you're doing the above local assignments all over the place in this file. Perhaps you could make a command that determines the log file path for you? Then each of the functions would shrink. Here's how it might look:
logpath() {
local app_name="$1"
local datetime=`date +%Y/%m/%d/%H`
local app_log_dir="{{ app_log_dir }}"
echo "${app_log_dir}/${datetime}/${app_name}.log"
}
lograw() {
tail -f "$(logpath $1)"
}|
Left comment on refactoring. Also, I suggest avoiding the use of the # This is bad
function stuff() { ... }
# This is good
stuff() { ... } |
| @@ -1,19 +1,51 @@ | |||
| # Follows the logfile for a given app_name interpolating the datetime string into the logpath (/var/log/runnable/YYYY/MM/DD/HH/<app_name>.log) | |||
| # Usage: logtail <app_name> | |||
There was a problem hiding this comment.
These comments should be above the logtail command. Also add doc above logpath to explain what it will do.
| # Usage: npmlog <app_name> | ||
| function npmlog() { | ||
| logtail() { | ||
| local app_name="$1" |
There was a problem hiding this comment.
Can you make these flexible and allow extra args to Bunyan?
$@
There was a problem hiding this comment.
That'd be awesome I'd love to logtail api-workers --level 40
There was a problem hiding this comment.
Everyone's a critic.
There was a problem hiding this comment.
@Myztiq @anandkumarpatel next rev gentlemen... until then you can tail -f $(logpath api) | bunyan [ <OPTS> ]
| if [ 2 -eq "{{ '${#}' }}" ] ; then | ||
| tailopts="-${2}" | ||
| fi | ||
| tail ${tailopts} $(logpath ${app_name}) | bunyan |
There was a problem hiding this comment.
Use quotes around ${tailopts} and $(logpath ...)
Reviewers
Tests
Deployment (post-merge)