On 8/1/24 00:53, Santiago Ruano Rincón wrote:
Hello Philipp,
El 30/07/24 a las 21:18, Philipp Hortmann escribió:
On 7/30/24 10:05, Santiago Ruano Rincón wrote:
Fix checkpatch error "ERROR: code indent should use tabs where possible"
in include/rtw_mlme_ext.h:388.
Signed-off-by: Santiago Ruano Rincón <santiagorr@xxxxxxxxxx>
---
I am (mostly) a newcommer. Could you please tell me if there is anything
wrong with this patch? Thank you!
checkpatch reports a warning in line 387:
"WARNING: line length of 135 exceeds 100 columns"
Should I fix the warning in the same patch, or should I send a second
patch to fix it?
---
---
drivers/staging/rtl8723bs/include/rtw_mlme_ext.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/rtl8723bs/include/rtw_mlme_ext.h b/drivers/staging/rtl8723bs/include/rtw_mlme_ext.h
index 5b8574f5a..720aeeb00 100644
--- a/drivers/staging/rtl8723bs/include/rtw_mlme_ext.h
+++ b/drivers/staging/rtl8723bs/include/rtw_mlme_ext.h
@@ -384,8 +384,8 @@ struct mlme_ext_priv {
unsigned char default_supported_mcs_set[16];
struct ss_res sitesurvey_res;
- struct mlme_ext_info mlmext_info;/* for sta/adhoc mode, including current scanning/connecting/connected related info. */
- /* for ap mode, network includes ap's cap_info */
+ struct mlme_ext_info mlmext_info; /* for sta/adhoc mode, including current scanning/connecting/connected related info. */
+ /* for ap mode, network includes ap's cap_info */
struct timer_list survey_timer;
struct timer_list link_timer;
struct timer_list sa_query_timer;
Hi Santiago,
it could be two patches or one. You are only allowed to do one thing at a
time. So it is also a little depending on your description. The way you
describe this it is more like two patches.
If your description would be like:
Insert line break to comment, that exceeds 100 columns, to improve
readability.
It would be OK to correct the indent as well.
If you send in a second version of this patch please use a change history.
Description from Dan under:
https://staticthinking.wordpress.com/2022/07/27/how-to-send-a-v2-patch/
In case of questions feel free to contact me directly.
Thanks for your support.
Bye Philipp
Thanks a lot for your explanation. Much appreciated.
Greg has added the commit to his staging-testing branch. So I guess
the only choice to fix that warning (a CHECK, actually) now is through a
separate commit.
That is correct.
Maybe several warnings/checks of the same type in the same file could be
handled together in a single commit?
Yes you can fix several issues in one commit if they are of same type.
Example: Missing linebreaks
But is it worth the effort to fix
them (when they are non-ERROR), or does it represent more work for you?
If you consider checkpatch warnings and errors it is more complicated.
We want to fix all errors and warnings when it makes sense. Sometimes
checkpatch is wrong. Finally you always need to judge if your patch is
an improvement for readability.
Example:
Checkpatch warnings about not needed brackets in logical statements are
most likely to be wrong. Reason is that the readability for those
logical statements is extremely lowered when omitting the brackets.
I cannot respond to you alone as Greg insists on all mails to be on the
mailing list.
https://lore.kernel.org/linux-staging/001101dae31f$d5ef44d0$81cdce70$@samsung.com/T/#m83a1aac71249e9d6b658bc5e4a7190c757d3547e
Thanks for your support.
Bye Philipp
