Code review comment for lp:~bryanfritt/compiz/compiz-decorator_script-edit_1192376

Revision history for this message
BryanFRitt (bryanfritt) wrote :

COMPIZ_BIN_PATH=$(cd $(dirname "$0"); vs COMPIZ_BIN_PATH=`dirname "$0"`
The first always gives the full path; The second changes based on how it's called. Put it in a script and it'll change based on how the script is called. Like is it called via a relative path, or via a full path? I suppose for this script this line could be done either way, although I haven't tested the script with the second.

you can try it out...
put this in a file and run different ways

# a quickly made example...
# with this code in /opt/CompizBZR/bin/pathTest.sh

THIS_PATH_1=$(cd $(dirname "$0"); pwd)
echo $THIS_PATH_1

THIS_PATH_2=`dirname "$0"`
echo $THIS_PATH_2

# results:
# called from /opt/CompizBZR/ with /opt/CompizBZR/bin/pathTests.sh
# /opt/CompizBZR/bin
# /opt/CompizBZR/bin

# also called from /opt/CompizBZR/ with ./bin/pathTests.sh
# /opt/CompizBZR/bin
# ./bin

# called from ~/ with ../../opt/CompizBZR/bin/pathTests.sh
# /opt/CompizBZR/bin
# ../../opt/CompizBZR/bin

# called from /opt/CompizBZR/bin/ with ./pathTests.sh
# ./pathTests.sh
# /opt/CompizBZR/bin

"There's lots of commented out code in the review version."
This was my first time messing with bzr, and Compiz submissions. I'm guessing I hit submit for review too soon?, and guessing it doesn't update which code to review? and so you might reviewed an earlier version?
(And I should resubmit again after changing tabs to 4 spaces?)

The one I should have submitted should have been done after 3751 `chmod +x`, the last thing I've done with this code. I hadn't thought of any changes to do with it since then, and it's working for me, so I guess I'm done with this set of edits, except for now messing with what you said, the using four spaces in place of a tab.

"These changes are inconsistent with the coding style. Use 8-wide tabs for two indents and 4-spaces for one indent, or, if the language doesn't support it, use 4-spaces for indents."
I didn't know about the coding style. Where can I read it?
Should be an easy change. No problem. It'll actually be easier to do copy paste code type testing on to the shell with spaces instead of tab. (tab characters cause 'tab completions' to activate during copy paste, which makes things look weird...)

"I'm not so sure how I feel about starting another _window manager_ just because a decorator wasn't found, but this code was here before so I don't mind too much."
I thought the same thing, and left it in there as a 'just in case', and 'it must have been added for a reason'..., If this code causes problems, it can be taken out without too much trouble.

« Back to merge proposal