nginx: update init script to publish services over mdns #23767
Conversation
|
please remove the merge commit @mhusaam |
|
Hi @systemcrash, thanks for pointing it out. I have updated the PR, kindly review. |
|
Hi @systemcrash, thanks for your comments. Kindly review. The following has been updated:
Thanks. |
net/nginx/files/nginx.init
Outdated
| port=$(echo "$port" | head -n1 | awk '{print $1;}') | ||
|
|
||
|
|
||
| if [ "${enabled}" != "0" ] && [ "${enabled}" != "false" ] && [ -n "${port}" ]; then |
There was a problem hiding this comment.
Any reason you chose "${...}" ? I see it used in a few other places in the file, but keep it to "$..." or ${...}.
Also, indent inside the if block.
Apply tabs consistently to indent inside this function block. Line up this function indent with the function above.
There was a problem hiding this comment.
Hello @systemcrash,
- ${} was me being "extra careful" in the in the test operator :)
- I like to have consistent indentation neither vim nor github show the indentation, and vim does not have great support for indentation in shell scripts (it is good for C), so I missed that. I will try to use VisualStudio editor for this in future.
net/nginx/files/nginx.init
Outdated
| configure_mdns() { | ||
| local mdns="$(uci -q get nginx.global.mdns)" | ||
|
|
||
| if [ "${mdns}" = "1" ] || [ "${mdns}" = "true" ]; then |
There was a problem hiding this comment.
Nice mix of styles here. [ ... ] is basically if. Not important.
It would improve readability to tab indent here inside the if block. Indent equally, using tabs.
So here, maybe outdent the if and fi to match local.
There was a problem hiding this comment.
Hello,
Fixed the indentation.
|
Otherwise, looks good functionality-wise. |
mhusaam
force-pushed
the
for_nginx_mdns
branch
5 times, most recently
from
bb06e3a to
3f265ca
Compare
|
Hello @systemcrash, thanks for your comments. I have resolved them. Kindly review. |
|
ping @Ansuel |
|
@Ra2-IFV thanks for the ping, will fix 1-2 formal thing and I will merge this. |
Update nginx init script to announce http related services over mdns. Signed-off-by: Mohd Husaam Mehdi <[email protected]> [ bump PKG release, improve commit description ] Signed-off-by: Christian Marangi <[email protected]>
Ansuel
force-pushed
the
for_nginx_mdns
branch
from
3f265ca to
d7e86e7
Compare
update init script to announce http related services over mdns
Signed-off-by: Mohd Husaam Mehdi [email protected]