[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