From: Mauro Carvalho Chehab <mchehab@s-opensource.com>
To: "Jasmin J." <jasmin@anw.at>
Cc: linux-media@vger.kernel.org, max.kellermann@gmail.com
Subject: Re: [PATCH 01/11] [media] dvb-core/dvb_ca_en50221.c: Rename STATUSREG_??
Date: Wed, 7 Jun 2017 13:43:19 -0300 [thread overview]
Message-ID: <20170607134319.6b90c6a4@vento.lan> (raw)
In-Reply-To: <3d4c4a10-0c65-9eee-b4e2-b19f1eddb31a@anw.at>
Em Mon, 8 May 2017 19:28:48 +0200
"Jasmin J." <jasmin@anw.at> escreveu:
> Hello Mauro!
>
> >> Rename STATUSREG_?? -> STATREG_?? to reduce the line length.
> > That sounds a bad idea. We use "stat" on the DVB subsystem as an
> > alias for statistics.
> At the beginning of the style fixes, I thought it is a good idea to reduce
> as much as possible to get more characters, but at the end this patch
> doesn't save so much, so we can omit it.
Renaming things is usually not a good idea, specially at core level,
as it makes a way harder to apply patches from other sources.
Also, if you're doing that just because of the 80cols warning, that's
the wrong way of doing it ;)
The hole idea when the 80cols warning was introduced is to point places at
the code were there are potentially too much indentation or code complexity,
possibly indicating complex functions that could otherwise be split.
This is useful when new code gets added, but it usually doesn't make
much sense to fix it on existing code, except when some function has to
be re-implemented.
So, I please drop patches 1 and 2 from this series.
> What is then the right procedure now?
> When I omit it in the first place, I can redo the whole work again and
> this were a lot of hours. Would it be acceptable to make a patch no. 12 at
> the end of the sequence, which renames it back?
If you want it applied, this is needed anyway, as the patch doesn't apply
cleanly:
patching file drivers/media/dvb-core/dvb_ca_en50221.c
Hunk #2 FAILED at 347.
Hunk #4 succeeded at 649 (offset -12 lines).
Hunk #5 succeeded at 702 (offset -11 lines).
Hunk #6 succeeded at 763 (offset -15 lines).
Hunk #7 succeeded at 779 (offset -15 lines).
Hunk #8 succeeded at 800 (offset -15 lines).
Hunk #9 succeeded at 824 (offset -15 lines).
Hunk #10 succeeded at 937 (offset -15 lines).
Hunk #11 succeeded at 1151 (offset -15 lines).
1 out of 11 hunks FAILED -- rejects in file drivers/media/dvb-core/dvb_ca_en50221.c
Patch patches/lmml_41185_01_11_media_dvb_core_dvb_ca_en50221_c_rename_statusreg.patch does not apply (enforce with -f)
Patch didn't apply. Aborting
>From this patch series, I was able to apply 2 patches.
Btw, don't spend time fixing issues pointed by checkpatch on existing
code, except if you're rewriting most of the code. We don't want to handle
merge conflicts due to checkpatch-only changes.
Thanks,
Mauro
next prev parent reply other threads:[~2017-06-07 16:43 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-05-07 21:23 [PATCH 00/11] Fix coding style in en50221 CAM functions Jasmin J.
2017-05-07 21:23 ` [PATCH 01/11] [media] dvb-core/dvb_ca_en50221.c: Rename STATUSREG_?? Jasmin J.
2017-05-08 9:55 ` Mauro Carvalho Chehab
2017-05-08 17:28 ` Jasmin J.
2017-06-07 16:43 ` Mauro Carvalho Chehab [this message]
2017-06-07 19:37 ` Jasmin J.
2017-06-07 22:59 ` Mauro Carvalho Chehab
2017-06-08 18:55 ` Jasmin J.
2017-05-07 21:23 ` [PATCH 02/11] [media] dvb-core/dvb_ca_en50221.c: Rename DVB_CA_SLOTSTATE_??? Jasmin J.
2017-05-07 21:23 ` [PATCH 03/11] [media] dvb-core/dvb_ca_en50221.c: Used a helper variable Jasmin J.
2017-05-07 21:23 ` [PATCH 04/11] [media] dvb-core/dvb_ca_en50221.c: Refactored dvb_ca_en50221_thread Jasmin J.
2017-05-07 21:23 ` [PATCH 05/11] [media] dvb-core/dvb_ca_en50221.c: Make checkpatch happy 1 Jasmin J.
2017-05-07 21:23 ` [PATCH 06/11] [media] dvb-core/dvb_ca_en50221.c: Make checkpatch happy 2 Jasmin J.
2017-05-07 21:23 ` [PATCH 07/11] [media] dvb-core/dvb_ca_en50221.c: Make checkpatch happy 3 Jasmin J.
2017-05-07 21:23 ` [PATCH 08/11] [media] dvb-core/dvb_ca_en50221.c: Make checkpatch happy 4 Jasmin J.
2017-05-07 21:23 ` [PATCH 09/11] [media] dvb-core/dvb_ca_en50221.c: Make checkpatch happy 5 Jasmin J.
2017-05-07 21:23 ` [PATCH 10/11] [media] dvb-core/dvb_ca_en50221.c: Make checkpatch happy 6 Jasmin J.
2017-05-07 21:23 ` [PATCH 11/11] [media] dvb-core/dvb_ca_en50221.c: Fixed wrong EXPORT_SYMBOL order Jasmin J.
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20170607134319.6b90c6a4@vento.lan \
--to=mchehab@s-opensource.com \
--cc=jasmin@anw.at \
--cc=linux-media@vger.kernel.org \
--cc=max.kellermann@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox