[Asterisk-code-review] install_prereq: Refactoring and	enhancements. (asterisk[master])
    Martin McCarthy 
    asteriskteam at digium.com
       
    Mon Mar  6 13:23:13 CST 2023
    
    
  
Attention is currently required from: Mark Murawski.
Martin McCarthy has posted comments on this change. ( https://gerrit.asterisk.org/c/asterisk/+/19939 )
Change subject: install_prereq: Refactoring and enhancements.
......................................................................
Patch Set 2:
(12 comments)
Patchset:
PS2: 
Thanks for the valuable feedback K.
File contrib/scripts/install_prereq:
https://gerrit.asterisk.org/c/asterisk/+/19939/comment/b6041cec_3313af76 
PS2, Line 7: installing
> Reformat for 80 cols
Ack
https://gerrit.asterisk.org/c/asterisk/+/19939/comment/eb302534_749dd24e 
PS2, Line 9: # - DragonFly
> Alphabetically sort this list
Ack
https://gerrit.asterisk.org/c/asterisk/+/19939/comment/ae946c85_71ea1cf2 
PS2, Line 25: # Function name: handle_debian
> The function name here is obvious.  Each comment block doesn't need to repeat the function name. […]
Ack
https://gerrit.asterisk.org/c/asterisk/+/19939/comment/f43d6826_e1675aa5 
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. […]
Ack
https://gerrit.asterisk.org/c/asterisk/+/19939/comment/b5116165_cb643078 
PS2, Line 30:     if ! [ -x "$(command -v aptitude)" ] ; then
> All indenting needs to be tabs
Ack
https://gerrit.asterisk.org/c/asterisk/+/19939/comment/0a673006_2c6accec 
PS2, Line 41:  
> Remove unnecessary 'x'.  Straight string comparison is sufficient here.
Ack
https://gerrit.asterisk.org/c/asterisk/+/19939/comment/40cfefd1_41762dcf 
PS2, Line 48:     # We're done!
> Don't need to write a 'done' comment for every return unless there's something special going on.
Ack
https://gerrit.asterisk.org/c/asterisk/+/19939/comment/3f097080_c1191d5e 
PS2, Line 58:     extra_packs=`check_installed_rpms $PACKAGES_RH`
> In the comment blocks for each function.  Document the prerequisites for the function. […]
Ack
https://gerrit.asterisk.org/c/asterisk/+/19939/comment/9f160bf1_70f937eb 
PS2, Line 60:     if [ x"$extra_packs" != "x" ] ; then
> Remove unnecessary 'x'.  Straight string comparison is sufficient here.
Ack
https://gerrit.asterisk.org/c/asterisk/+/19939/comment/23a05624_20b2a8a0 
PS2, Line 79:     if [ x"$extra_packs" != "x" ] ; then
> Same as above.  Please fix all of these.
Ack
https://gerrit.asterisk.org/c/asterisk/+/19939/comment/69f9b1a8_2e5422b6 
PS2, Line 303:         
> Fix coding style […]
Ack
-- 
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: Mark Murawski <markm at intellasoft.net>
Gerrit-Comment-Date: Mon, 06 Mar 2023 19:23:13 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Mark Murawski <markm at intellasoft.net>
Gerrit-MessageType: comment
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20230306/0b411f22/attachment-0001.html>
    
    
More information about the asterisk-code-review
mailing list