[Asterisk-code-review] install_prereq: Refactoring and enhancements. (asterisk[master])

[Mark Murawski]

Attention is currently required from: Martin McCarthy.

Mark Murawski has posted comments on this change. ( https://gerrit.asterisk.org/c/asterisk/+/19939 )

Change subject: install_prereq: Refactoring and enhancements.
......................................................................


Patch Set 2: Code-Review-1

(11 comments)

File contrib/scripts/install_prereq:

https://gerrit.asterisk.org/c/asterisk/+/19939/comment/17c10524_b545f5a9 
PS2, Line 7: installing
Reformat for 80 cols


https://gerrit.asterisk.org/c/asterisk/+/19939/comment/2fd7ca15_d49323ca 
PS2, Line 9: # - DragonFly
Alphabetically sort this list


https://gerrit.asterisk.org/c/asterisk/+/19939/comment/f479b7b2_7b3f31ca 
PS2, Line 25: # Function name: handle_debian
The function name here is obvious.  Each comment block doesn't need to repeat the function name.  Same goes for all the others.


https://gerrit.asterisk.org/c/asterisk/+/19939/comment/cd7c71c8_71a29b7f 
PS2, Line 26: # Date: 01/11/22
I don't think it's necessary to date every function here.  This is not a library/api file.  Same goes for all the others.


https://gerrit.asterisk.org/c/asterisk/+/19939/comment/c58c50a2_d755fd73 
PS2, Line 30:     if ! [ -x "$(command -v aptitude)" ] ; then
All indenting needs to be tabs


https://gerrit.asterisk.org/c/asterisk/+/19939/comment/e3787018_9f0e8a31 
PS2, Line 41:  
Remove unnecessary 'x'.  Straight string comparison is sufficient here.


https://gerrit.asterisk.org/c/asterisk/+/19939/comment/ae88bd01_32ebace2 
PS2, Line 48:     # We're done!
Don't need to write a 'done' comment for every return unless there's something special going on.


https://gerrit.asterisk.org/c/asterisk/+/19939/comment/cf91ba8e_0ddd83b9 
PS2, Line 58:     extra_packs=`check_installed_rpms $PACKAGES_RH`
In the comment blocks for each function.  Document the prerequisites for the function.  Describe what $PACKAGES_RH is supposed to be prior to the handle_rh() call

Same goes for all the other functions


https://gerrit.asterisk.org/c/asterisk/+/19939/comment/77f7522c_7d4e18de 
PS2, Line 60:     if [ x"$extra_packs" != "x" ] ; then
Remove unnecessary 'x'.  Straight string comparison is sufficient here.


https://gerrit.asterisk.org/c/asterisk/+/19939/comment/b12b10ee_379eb977 
PS2, Line 79:     if [ x"$extra_packs" != "x" ] ; then
Same as above.  Please fix all of these.


https://gerrit.asterisk.org/c/asterisk/+/19939/comment/baf4c874_a6dbd076 
PS2, Line 303:         
Fix coding style

; if [ condition ]; then
;   dostuff
; fi



-- 
To view, visit https://gerrit.asterisk.org/c/asterisk/+/19939
To unsubscribe, or for help writing mail filters, visit https://gerrit.asterisk.org/settings

Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Change-Id: I4bd53ae429e113a76d13a23b48da714a8eefad26
Gerrit-Change-Number: 19939
Gerrit-PatchSet: 2
Gerrit-Owner: Martin McCarthy <martin.c.mccarthy at outlook.com>
Gerrit-Reviewer: Friendly Automation
Gerrit-Reviewer: Mark Murawski <markm at intellasoft.net>
Gerrit-Attention: Martin McCarthy <martin.c.mccarthy at outlook.com>
Gerrit-Comment-Date: Mon, 06 Mar 2023 18:47:30 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20230306/a383e70c/attachment.html>