Re: [OpenWrt-Devel] [PATCH v3] base-files: make wifi report unknown command

2018-08-10 Thread Jonas Gorski
On 9 August 2018 at 20:35, Thibaut  wrote:
>
>> On 9 Aug 2018, at 19:58, Jonas Gorski  wrote:
>>
>> On 9 August 2018 at 18:49, Thibaut VARÈNE  wrote:
>>> Avoid having /sbin/wifi silently ignore unknown keywords and execute
>>> "enable"; instead display the help message and exit with an error.
>>>
>>> Spell out the 'enable' keyword and preserve the implicit assumption
>>> that runing /sbin/wifi without argument performs "enable".
>>>
>>> Signed-off-by: Thibaut VARÈNE 
>>> ---
>>
>> Please keep a changelog here to know what's changed between submissions.
>
> it’s a 2-liner patch. I don’t think it warrants a changelog in the commit 
> message :)

The fact that you needed to send 4 versions should be enough of an
argument to keep a changelog regardless of the size of the patch ;)

Also the changelog belongs below the tear-off line ("---") so it
doesn't end in the commit message.

So please keep that in mind for future patches.


Regards
Jonas

___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel


Re: [OpenWrt-Devel] [PATCH v3] base-files: make wifi report unknown command

2018-08-09 Thread Thibaut

> On 9 Aug 2018, at 19:58, Jonas Gorski  wrote:
> 
> On 9 August 2018 at 18:49, Thibaut VARÈNE  wrote:
>> Avoid having /sbin/wifi silently ignore unknown keywords and execute
>> "enable"; instead display the help message and exit with an error.
>> 
>> Spell out the 'enable' keyword and preserve the implicit assumption
>> that runing /sbin/wifi without argument performs "enable".
>> 
>> Signed-off-by: Thibaut VARÈNE 
>> ---
> 
> Please keep a changelog here to know what's changed between submissions.

it’s a 2-liner patch. I don’t think it warrants a changelog in the commit 
message :)

v4: use “up” instead of “enable” and add it to usage output.

Cheers,
T.
___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel


Re: [OpenWrt-Devel] [PATCH v3] base-files: make wifi report unknown command

2018-08-09 Thread Jonas Gorski
On 9 August 2018 at 18:49, Thibaut VARÈNE  wrote:
> Avoid having /sbin/wifi silently ignore unknown keywords and execute
> "enable"; instead display the help message and exit with an error.
>
> Spell out the 'enable' keyword and preserve the implicit assumption
> that runing /sbin/wifi without argument performs "enable".
>
> Signed-off-by: Thibaut VARÈNE 
> ---

Please keep a changelog here to know what's changed between submissions.

>  package/base-files/files/sbin/wifi | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/package/base-files/files/sbin/wifi 
> b/package/base-files/files/sbin/wifi
> index 83befc0d6f..68536ed25f 100755
> --- a/package/base-files/files/sbin/wifi
> +++ b/package/base-files/files/sbin/wifi
> @@ -241,5 +241,6 @@ case "$1" in
> reload) wifi_reload "$2";;
> reload_legacy) wifi_reload_legacy "$2";;
> --help|help) usage;;
> -   *) ubus call network reload; wifi_updown "enable" "$2";;
> +   ''|enable) ubus call network reload; wifi_updown "enable" "$2";;

Not that I particularily want to bikeshed here, but the disable
command is "down", so the enable command should be "up", not "enable".

There are actually existing users of "up": see

https://github.com/openwrt/openwrt/blob/master/package/network/config/netifd/files/sbin/ifup#L43

and

https://github.com/openwrt/openwrt/blob/master/package/network/config/netifd/files/sbin/ifup#L75

or

https://github.com/openwrt/openwrt/blob/master/package/base-files/files/etc/rc.button/rfkill#L30

so it isn't just a should, it's a must. Or the existing users must be
changed before.

> +   *) usage; exit 1;;
>  esac


Regards
Jonas

___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel


[OpenWrt-Devel] [PATCH v3] base-files: make wifi report unknown command

2018-08-09 Thread Thibaut VARÈNE
Avoid having /sbin/wifi silently ignore unknown keywords and execute
"enable"; instead display the help message and exit with an error.

Spell out the 'enable' keyword and preserve the implicit assumption
that runing /sbin/wifi without argument performs "enable".

Signed-off-by: Thibaut VARÈNE 
---
 package/base-files/files/sbin/wifi | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/package/base-files/files/sbin/wifi 
b/package/base-files/files/sbin/wifi
index 83befc0d6f..68536ed25f 100755
--- a/package/base-files/files/sbin/wifi
+++ b/package/base-files/files/sbin/wifi
@@ -241,5 +241,6 @@ case "$1" in
reload) wifi_reload "$2";;
reload_legacy) wifi_reload_legacy "$2";;
--help|help) usage;;
-   *) ubus call network reload; wifi_updown "enable" "$2";;
+   ''|enable) ubus call network reload; wifi_updown "enable" "$2";;
+   *) usage; exit 1;;
 esac
-- 
2.14.3 (Apple Git-98)


___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel