Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2103005:
Add script to collect logs, dumps and other relevant information. (Closed)
DescriptionAdd script to collect logs, dumps and other relevant information.
Patch Set 1 #
Total comments: 27
Patch Set 2 : Changes to generate_logs as per review comments #Patch Set 3 : Changes to generate_logs to remove - from echo statements #
Total comments: 17
Patch Set 4 : Changes round 2 to script generate_logs as per review comments #Patch Set 5 : Changes to remove Wifi adapter info #
Total comments: 18
Patch Set 6 : Changes to generate_logs for optimization #Patch Set 7 : Changes to generate_logs script for optimization #
Total comments: 5
Patch Set 8 : More changes for optimization #
Total comments: 1
Messages
Total messages: 20 (0 generated)
http://codereview.chromium.org/2103005/diff/1/2 File src/platform/metrics/generate_logs (right): http://codereview.chromium.org/2103005/diff/1/2#newcode9 src/platform/metrics/generate_logs:9: One one line needed http://codereview.chromium.org/2103005/diff/1/2#newcode11 src/platform/metrics/generate_logs:11: if [ ! -z $1 ] && ([ $1 = -h ] || [ $1 = -? ]); then Use -n and not ! -z http://codereview.chromium.org/2103005/diff/1/2#newcode12 src/platform/metrics/generate_logs:12: echo "Usage of generate_logs.sh script - " Remove .sh here in echo http://codereview.chromium.org/2103005/diff/1/2#newcode13 src/platform/metrics/generate_logs:13: echo "To collect data - logs, dumps and screen-shots: bash generate_logs.sh " no - or .sh http://codereview.chromium.org/2103005/diff/1/2#newcode14 src/platform/metrics/generate_logs:14: echo "To collect data and clear up folders: bash generate_logs.sh -delete" .sh http://codereview.chromium.org/2103005/diff/1/2#newcode15 src/platform/metrics/generate_logs:15: echo "Collected data is zipped to tar.bz2 file with timestamp in ~/Downloads" This won't work on non-developer / non-test images because tar will not be included. Can you use something else other than tar? http://codereview.chromium.org/2103005/diff/1/2#newcode21 src/platform/metrics/generate_logs:21: if [ ! -d ~/Downloads ]; then You should probably use /home/$USER/user/Downloads and not ~. This way you can call this script from vt02. I do not think we're keeping around aterm on non dev builds which means you'll never have $CHROMEOS_USER from a terminal. Save this somewhere and replace all your ~'s throughout the script http://codereview.chromium.org/2103005/diff/1/2#newcode36 src/platform/metrics/generate_logs:36: dmesg | grep iwl | grep "Detected" > ~/Downloads/diagnostic_logs/wifi_adapter.txt > 80 chars. http://codereview.chromium.org/2103005/diff/1/2#newcode47 src/platform/metrics/generate_logs:47: mkdir ~/Downloads/diagnostic_logs/window_manager Combine this two lines to cp -rf /var/log/window_manager http://codereview.chromium.org/2103005/diff/1/2#newcode51 src/platform/metrics/generate_logs:51: # Copying screen-shots and deleting from original location You don't actually delete them. Did you mean to use mv and not cp http://codereview.chromium.org/2103005/diff/1/2#newcode53 src/platform/metrics/generate_logs:53: cp ~/Downloads/Screenshots/* ~/Downloads/diagnostic_logs/. Again use cp -rf and not use * and . http://codereview.chromium.org/2103005/diff/1/2#newcode57 src/platform/metrics/generate_logs:57: # Copying crash dumps and deleting from original location Same here http://codereview.chromium.org/2103005/diff/1/2#newcode65 src/platform/metrics/generate_logs:65: tar cjfP ~/Downloads/log-$currentdate.tar.bz2 ~/Downloads/diagnostic_logs/ Not in base system. Do you need to tar it?
Please see the changes made to the script and let me know of the corrections required. Thanks, Ruchi http://codereview.chromium.org/2103005/diff/1/2 File src/platform/metrics/generate_logs (right): http://codereview.chromium.org/2103005/diff/1/2#newcode9 src/platform/metrics/generate_logs:9: On 2010/05/17 22:14:08, sosa wrote: Done. http://codereview.chromium.org/2103005/diff/1/2#newcode11 src/platform/metrics/generate_logs:11: if [ ! -z $1 ] && ([ $1 = -h ] || [ $1 = -? ]); then On 2010/05/17 22:14:08, sosa wrote: -n does not seem to work here as _blank_ is not the same as "" http://codereview.chromium.org/2103005/diff/1/2#newcode12 src/platform/metrics/generate_logs:12: echo "Usage of generate_logs.sh script - " On 2010/05/17 22:14:08, sosa wrote: Done. http://codereview.chromium.org/2103005/diff/1/2#newcode13 src/platform/metrics/generate_logs:13: echo "To collect data - logs, dumps and screen-shots: bash generate_logs.sh " On 2010/05/17 22:14:08, sosa wrote: Done. http://codereview.chromium.org/2103005/diff/1/2#newcode14 src/platform/metrics/generate_logs:14: echo "To collect data and clear up folders: bash generate_logs.sh -delete" On 2010/05/17 22:14:08, sosa wrote: Done. http://codereview.chromium.org/2103005/diff/1/2#newcode15 src/platform/metrics/generate_logs:15: echo "Collected data is zipped to tar.bz2 file with timestamp in ~/Downloads" On 2010/05/17 22:14:08, sosa wrote: As discussed offline, its ok to use tar for now. http://codereview.chromium.org/2103005/diff/1/2#newcode21 src/platform/metrics/generate_logs:21: if [ ! -d ~/Downloads ]; then On 2010/05/17 22:14:08, sosa wrote: Done. http://codereview.chromium.org/2103005/diff/1/2#newcode36 src/platform/metrics/generate_logs:36: dmesg | grep iwl | grep "Detected" > ~/Downloads/diagnostic_logs/wifi_adapter.txt On 2010/05/17 22:14:08, sosa wrote: Done. http://codereview.chromium.org/2103005/diff/1/2#newcode47 src/platform/metrics/generate_logs:47: mkdir ~/Downloads/diagnostic_logs/window_manager On 2010/05/17 22:14:08, sosa wrote: Done. http://codereview.chromium.org/2103005/diff/1/2#newcode51 src/platform/metrics/generate_logs:51: # Copying screen-shots and deleting from original location On 2010/05/17 22:14:08, sosa wrote: Code flow had changed but had missed updating the comment. Updated it now. Thanks! http://codereview.chromium.org/2103005/diff/1/2#newcode51 src/platform/metrics/generate_logs:51: # Copying screen-shots and deleting from original location On 2010/05/17 22:14:08, sosa wrote: Done. http://codereview.chromium.org/2103005/diff/1/2#newcode53 src/platform/metrics/generate_logs:53: cp ~/Downloads/Screenshots/* ~/Downloads/diagnostic_logs/. On 2010/05/17 22:14:08, sosa wrote: Done. http://codereview.chromium.org/2103005/diff/1/2#newcode57 src/platform/metrics/generate_logs:57: # Copying crash dumps and deleting from original location On 2010/05/17 22:14:08, sosa wrote: Done. http://codereview.chromium.org/2103005/diff/1/2#newcode65 src/platform/metrics/generate_logs:65: tar cjfP ~/Downloads/log-$currentdate.tar.bz2 ~/Downloads/diagnostic_logs/ On 2010/05/17 22:14:08, sosa wrote: As discussed offline, its ok to use tar for now.
A few random nits... http://codereview.chromium.org/2103005/diff/8001/9001 File src/platform/metrics/generate_logs (right): http://codereview.chromium.org/2103005/diff/8001/9001#newcode10 src/platform/metrics/generate_logs:10: if [ ! -z $1 ] && ([ $1 = -h ] || [ $1 = -? ]); then Use "$1" -- otherwise passing ./generate_logs "foo bar" would break your script. http://codereview.chromium.org/2103005/diff/8001/9001#newcode10 src/platform/metrics/generate_logs:10: if [ ! -z $1 ] && ([ $1 = -h ] || [ $1 = -? ]); then Isn't ! -z equivalent to -n ? And then you probably don't need the check if you use "$1" http://codereview.chromium.org/2103005/diff/8001/9001#newcode12 src/platform/metrics/generate_logs:12: echo "To collect logs, dumps and screenshots: sh generate_logs" Do you need "sh" here? http://codereview.chromium.org/2103005/diff/8001/9001#newcode18 src/platform/metrics/generate_logs:18: if [ -z $USER ]; then Why do you need this? Are you expecting to collect logs for root? That won't work anyway... I'd hardcode chronos -- it's hardcoded it many places already anyway. http://codereview.chromium.org/2103005/diff/8001/9001#newcode26 src/platform/metrics/generate_logs:26: if [ ! -d $home_dir/Downloads ]; then These 8 lines can be replaced by 2: mkdir -p $home_dir/Downloads/diagnostic_logs sudo rm -rf $home_dir/Downloads/diagnostic_logs/* http://codereview.chromium.org/2103005/diff/8001/9001#newcode41 src/platform/metrics/generate_logs:41: dmesg | grep iwl | grep "Detected" > \ Do you need both dmesg and /var/log/messages below? Isn't /var/log/messages enough? Maybe it's not, just asking... http://codereview.chromium.org/2103005/diff/8001/9001#newcode54 src/platform/metrics/generate_logs:54: $home_dir/Downloads/diagnostic_logs/window_manager For consistency, no need to specify "window_manager" in the target path, I think.
http://codereview.chromium.org/2103005/diff/8001/9001 File src/platform/metrics/generate_logs (right): http://codereview.chromium.org/2103005/diff/8001/9001#newcode10 src/platform/metrics/generate_logs:10: if [ ! -z $1 ] && ([ $1 = -h ] || [ $1 = -? ]); then It isn't in this case. It only is if you explicitly do something like $1="$1" in case $1 isn't set at all. I had the same concern originally. On 2010/05/21 18:05:35, petkov wrote: > Isn't ! -z equivalent to -n ? And then you probably don't need the check if you > use "$1" > http://codereview.chromium.org/2103005/diff/8001/9001#newcode18 src/platform/metrics/generate_logs:18: if [ -z $USER ]; then Agreed. Just use $USER. It's impossible to call this script as a user without that variable set. On 2010/05/21 18:05:35, petkov wrote: > Why do you need this? Are you expecting to collect logs for root? That won't > work anyway... I'd hardcode chronos -- it's hardcoded it many places already > anyway. > >
http://codereview.chromium.org/2103005/diff/8001/9001 File src/platform/metrics/generate_logs (right): http://codereview.chromium.org/2103005/diff/8001/9001#newcode10 src/platform/metrics/generate_logs:10: if [ ! -z $1 ] && ([ $1 = -h ] || [ $1 = -? ]); then You mean that it's not equivalent to [ -n "$1" ] ? The code should use quotes around $1 everywhere anyway. On 2010/05/24 20:12:23, sosa wrote: > It isn't in this case. It only is if you explicitly do something like $1="$1" > in case $1 isn't set at all. I had the same concern originally. > > On 2010/05/21 18:05:35, petkov wrote: > > Isn't ! -z equivalent to -n ? And then you probably don't need the check if > you > > use "$1" > > > >
Please see the changes made to the script. Thanks, Ruchi http://codereview.chromium.org/2103005/diff/8001/9001 File src/platform/metrics/generate_logs (right): http://codereview.chromium.org/2103005/diff/8001/9001#newcode10 src/platform/metrics/generate_logs:10: if [ ! -z $1 ] && ([ $1 = -h ] || [ $1 = -? ]); then On 2010/05/21 18:05:35, petkov wrote: Done. http://codereview.chromium.org/2103005/diff/8001/9001#newcode10 src/platform/metrics/generate_logs:10: if [ ! -z $1 ] && ([ $1 = -h ] || [ $1 = -? ]); then On 2010/05/24 20:39:28, petkov wrote: [ -n "$1" ]works. Thanks. http://codereview.chromium.org/2103005/diff/8001/9001#newcode12 src/platform/metrics/generate_logs:12: echo "To collect logs, dumps and screenshots: sh generate_logs" On 2010/05/21 18:05:35, petkov wrote: Changed to {path to script}/generate_logs. Please confirm if that looks ok. http://codereview.chromium.org/2103005/diff/8001/9001#newcode18 src/platform/metrics/generate_logs:18: if [ -z $USER ]; then On 2010/05/24 20:12:23, sosa wrote: If the script is run from normal terminal, then it doesn't work as $USER is not set. However, since users won't have terminal and will use VT where $USER is set to chronos, thus removed above check. Done http://codereview.chromium.org/2103005/diff/8001/9001#newcode26 src/platform/metrics/generate_logs:26: if [ ! -d $home_dir/Downloads ]; then On 2010/05/21 18:05:35, petkov wrote: Done. http://codereview.chromium.org/2103005/diff/8001/9001#newcode41 src/platform/metrics/generate_logs:41: dmesg | grep iwl | grep "Detected" > \ On 2010/05/21 18:05:35, petkov wrote: Done http://codereview.chromium.org/2103005/diff/8001/9001#newcode54 src/platform/metrics/generate_logs:54: $home_dir/Downloads/diagnostic_logs/window_manager On 2010/05/21 18:05:35, petkov wrote: Thanks for pointing out. Restructured the folders a bit as suggested by Kris. Under diagnostics logs, there are - 1) files like - timestamp, wifi_adapter and lsb-release. 2) user_level_logs - with screenlocker logs and windows manager logs of ~/log/ 3) system_level_logs - with logs from /var/log 4) screenshots 5) crash dumps Please let me know if it looks ok to you
cpl nits from me, o/w lgtm from me. but hold off till petkov lgtm's as well. http://codereview.chromium.org/2103005/diff/19001/20001 File src/platform/metrics/generate_logs (right): http://codereview.chromium.org/2103005/diff/19001/20001#newcode70 src/platform/metrics/generate_logs:70: if [ -z "$1" ]; then Don't need this. Do if [$1 = --delete] else print out echos http://codereview.chromium.org/2103005/diff/19001/20001#newcode73 src/platform/metrics/generate_logs:73: elif [ "$1" = -delete ]; then Use --delete not -delete
On 2010/05/26 01:16:47, sosa wrote: > cpl nits from me, o/w lgtm from me. but hold off till petkov lgtm's as well. > > http://codereview.chromium.org/2103005/diff/19001/20001 > File src/platform/metrics/generate_logs (right): > > http://codereview.chromium.org/2103005/diff/19001/20001#newcode70 > src/platform/metrics/generate_logs:70: if [ -z "$1" ]; then > Don't need this. Do if [$1 = --delete] else print out echos > > http://codereview.chromium.org/2103005/diff/19001/20001#newcode73 > src/platform/metrics/generate_logs:73: elif [ "$1" = -delete ]; then > Use --delete not -delete Please confirm if I should change -h and -? for help to --h and --? also.
No only multiletter options are --. -h an -? are one - because they are single character options On Tue, May 25, 2010 at 9:38 PM, <ruchic@chromium.org> wrote: > On 2010/05/26 01:16:47, sosa wrote: >> >> cpl nits from me, o/w lgtm from me. but hold off till petkov lgtm's as >> well. > >> http://codereview.chromium.org/2103005/diff/19001/20001 >> File src/platform/metrics/generate_logs (right): > >> http://codereview.chromium.org/2103005/diff/19001/20001#newcode70 >> src/platform/metrics/generate_logs:70: if [ -z "$1" ]; then >> Don't need this. Do if [$1 = --delete] else print out echos > >> http://codereview.chromium.org/2103005/diff/19001/20001#newcode73 >> src/platform/metrics/generate_logs:73: elif [ "$1" = -delete ]; then >> Use --delete not -delete > > Please confirm if I should change -h and -? for help to --h and --? also. > > http://codereview.chromium.org/2103005/show >
Again, just nits. It's just the script looks a bit too long and verbose for what it's doing. http://codereview.chromium.org/2103005/diff/19001/20001 File src/platform/metrics/generate_logs (right): http://codereview.chromium.org/2103005/diff/19001/20001#newcode10 src/platform/metrics/generate_logs:10: if [ -n "$1" ] && ([ "$1" = -h ] || [ "$1" = -? ]); then I'm not a shell script expert, but do you really need the [-n "$1"] here? http://codereview.chromium.org/2103005/diff/19001/20001#newcode12 src/platform/metrics/generate_logs:12: echo "To collect logs, dumps and screenshots: {path_to_script}/generate_logs" Isn't {path_to_script}/generate_logs just $0 ? Same below. http://codereview.chromium.org/2103005/diff/19001/20001#newcode23 src/platform/metrics/generate_logs:23: mkdir -p $home_dir/Downloads/diagnostic_logs Assign $home_dir/Downloads/diagnostic_logs to a variable and use throughout. http://codereview.chromium.org/2103005/diff/19001/20001#newcode36 src/platform/metrics/generate_logs:36: cp -rf /var/log/window_manager \ Given that you are line wrapping anyway, you could combine these few lines into a single command: cp -rf \ bar \ foo \ zoo \ target_directory ... and keep the sources sorted. http://codereview.chromium.org/2103005/diff/19001/20001#newcode49 src/platform/metrics/generate_logs:49: if [ -d $home_dir/Downloads/Screenshots ]; then cp -rf $home_dir/Downloads/Screenshots $home_dir/Downloads/diagnostic_logs/screenshots 2> /dev/null && echo "Copied screen-shots" same for crash reports http://codereview.chromium.org/2103005/diff/19001/20001#newcode75 src/platform/metrics/generate_logs:75: sudo rm -rf /var/log/messages Again, do these in a single command. http://codereview.chromium.org/2103005/diff/19001/20001#newcode89 src/platform/metrics/generate_logs:89: echo "Rebooting system..after clean up in 1 minute" s/../ with something else (e.g., "... ")
Please see the changes made to the script. Thanks, Ruchi http://codereview.chromium.org/2103005/diff/19001/20001 File src/platform/metrics/generate_logs (right): http://codereview.chromium.org/2103005/diff/19001/20001#newcode10 src/platform/metrics/generate_logs:10: if [ -n "$1" ] && ([ "$1" = -h ] || [ "$1" = -? ]); then On 2010/05/26 05:05:02, petkov wrote: > I'm not a shell script expert, but do you really need the [-n "$1"] here? > Done. http://codereview.chromium.org/2103005/diff/19001/20001#newcode12 src/platform/metrics/generate_logs:12: echo "To collect logs, dumps and screenshots: {path_to_script}/generate_logs" On 2010/05/26 05:05:02, petkov wrote: > Isn't {path_to_script}/generate_logs just $0 ? Same below. > > Done. http://codereview.chromium.org/2103005/diff/19001/20001#newcode23 src/platform/metrics/generate_logs:23: mkdir -p $home_dir/Downloads/diagnostic_logs On 2010/05/26 05:05:02, petkov wrote: > Assign $home_dir/Downloads/diagnostic_logs to a variable and use throughout. > Done. http://codereview.chromium.org/2103005/diff/19001/20001#newcode36 src/platform/metrics/generate_logs:36: cp -rf /var/log/window_manager \ On 2010/05/26 05:05:02, petkov wrote: > Given that you are line wrapping anyway, you could combine these few lines into > a single command: > > cp -rf \ > bar \ > foo \ > zoo \ > target_directory > > ... and keep the sources sorted. > Done. http://codereview.chromium.org/2103005/diff/19001/20001#newcode49 src/platform/metrics/generate_logs:49: if [ -d $home_dir/Downloads/Screenshots ]; then On 2010/05/26 05:05:02, petkov wrote: > cp -rf $home_dir/Downloads/Screenshots > $home_dir/Downloads/diagnostic_logs/screenshots 2> /dev/null && echo "Copied > screen-shots" > > same for crash reports > > Done. http://codereview.chromium.org/2103005/diff/19001/20001#newcode70 src/platform/metrics/generate_logs:70: if [ -z "$1" ]; then On 2010/05/26 01:16:47, sosa wrote: > Don't need this. Do if [$1 = --delete] else print out echos Done. http://codereview.chromium.org/2103005/diff/19001/20001#newcode73 src/platform/metrics/generate_logs:73: elif [ "$1" = -delete ]; then On 2010/05/26 01:16:47, sosa wrote: > Use --delete not -delete Done. http://codereview.chromium.org/2103005/diff/19001/20001#newcode75 src/platform/metrics/generate_logs:75: sudo rm -rf /var/log/messages On 2010/05/26 05:05:02, petkov wrote: > Again, do these in a single command. > Done. http://codereview.chromium.org/2103005/diff/19001/20001#newcode89 src/platform/metrics/generate_logs:89: echo "Rebooting system..after clean up in 1 minute" On 2010/05/26 05:05:02, petkov wrote: > s/../ with something else (e.g., "... ") > Done.
LGTM with a couple of nits. Please address at least the first one before pushing. http://codereview.chromium.org/2103005/diff/30001/31001 File src/platform/metrics/generate_logs (right): http://codereview.chromium.org/2103005/diff/30001/31001#newcode23 src/platform/metrics/generate_logs:23: log_dir=$home_dir/Downloads/diagnostic_logs You should assign log_dir first, and then mkdir -p $log_dir http://codereview.chromium.org/2103005/diff/30001/31001#newcode37 src/platform/metrics/generate_logs:37: cp -r /var/log/messages \ You could assign all logs to a variable (e.g., SYSLOGS) and use it both here and in the delete step below. Then adding more system level logs in the future would require just a single line update.
http://codereview.chromium.org/2103005/diff/30001/31001 File src/platform/metrics/generate_logs (right): http://codereview.chromium.org/2103005/diff/30001/31001#newcode37 src/platform/metrics/generate_logs:37: cp -r /var/log/messages \ In principal I agree with the maintainability comment, however, since this script is a stop-gap before it is integrated as a feature in user feedback, I don't think this is necessary. Also the "for and in" logic which I think would make this nice and maintainable is a bash-ism that they can't use since this needs to be a simple sh script that runs under dash. On 2010/05/27 04:22:07, petkov wrote: > You could assign all logs to a variable (e.g., SYSLOGS) and use it both here and > in the delete step below. Then adding more system level logs in the future would > require just a single line update. >
1: I've already removed xterm from base images so this isn't a concern. 2: Check in the script with the correct permissions (i.e make sure the script is 0755 in the source tree) before you commit and the permissions will be correct. On Thu, May 27, 2010 at 10:19 AM, Ruchi Chaturvedi <ruchic@chromium.org> wrote: > Before I upload the change, I wanted to give heads up about 2 points- > > Script works in VT only and not in normal terminal as per changes that I > was asked to made in line 18 of > http://codereview.chromium.org/2103005/diff2/8001:30001/31001 > > Do you prefer I hardcode to 'chronos' instead of $USER so that it works on > normal terminal also ? > > When I download the script from mails, I need to do chmod before I can run > it locally. > > Will permissions on the script be handled automatically OR do I need to do > some changes before the script is submitted ? > > Regards, > Ruchi > > On Thu, May 27, 2010 at 10:02 AM, <sosa@chromium.org> wrote: >> >> http://codereview.chromium.org/2103005/diff/30001/31001 >> File src/platform/metrics/generate_logs (right): >> >> http://codereview.chromium.org/2103005/diff/30001/31001#newcode37 >> src/platform/metrics/generate_logs:37: cp -r /var/log/messages \ >> In principal I agree with the maintainability comment, however, since >> this script is a stop-gap before it is integrated as a feature in user >> feedback, I don't think this is necessary. Also the "for and in" logic >> which I think would make this nice and maintainable is a bash-ism that >> they can't use since this needs to be a simple sh script that runs under >> dash. >> >> On 2010/05/27 04:22:07, petkov wrote: >>> >>> You could assign all logs to a variable (e.g., SYSLOGS) and use it >> >> both here and >>> >>> in the delete step below. Then adding more system level logs in the >> >> future would >>> >>> require just a single line update. >> >> >> http://codereview.chromium.org/2103005/show > >
Thanks Chris and Darin for insightful comments. Regards, Ruchi http://codereview.chromium.org/2103005/diff/30001/31001 File src/platform/metrics/generate_logs (right): http://codereview.chromium.org/2103005/diff/30001/31001#newcode23 src/platform/metrics/generate_logs:23: log_dir=$home_dir/Downloads/diagnostic_logs On 2010/05/27 04:22:07, petkov wrote: > You should assign log_dir first, and then mkdir -p $log_dir > Done. http://codereview.chromium.org/2103005/diff/36001/37001#newcode12 src/platform/metrics/generate_logs:12: echo "To collect logs, dumps and screenshots: $0" As discussed with Chris, there is some issue in bash configuration and thus we need to do sh to run the script.
still LGTM http://codereview.chromium.org/2103005/diff/30001/31001 File src/platform/metrics/generate_logs (right): http://codereview.chromium.org/2103005/diff/30001/31001#newcode37 src/platform/metrics/generate_logs:37: cp -r /var/log/messages \ Petkov actually corrected me. I imagined he wanted for / in, where he was just saying use cp $var $dst where var was ws separated list of items. Same with rm. On 2010/05/27 17:02:03, sosa wrote: > In principal I agree with the maintainability comment, however, since this > script is a stop-gap before it is integrated as a feature in user feedback, I > don't think this is necessary. Also the "for and in" logic which I think would > make this nice and maintainable is a bash-ism that they can't use since this > needs to be a simple sh script that runs under dash. > > On 2010/05/27 04:22:07, petkov wrote: > > You could assign all logs to a variable (e.g., SYSLOGS) and use it both here > and > > in the delete step below. Then adding more system level logs in the future > would > > require just a single line update. > > > >
Chris can you please commit the CL. Thanks, Ruchi
|
