[alsa-devel] [PATCH 00/44] remove unnecessary semicolons
ya trivial series...
Joe Perches (44): arch/arm: Remove unnecessary semicolons arch/microblaze: Remove unnecessary semicolons arch/um: Remove unnecessary semicolons drivers/cpufreq: Remove unnecessary semicolons drivers/gpio: Remove unnecessary semicolons drivers/i2c: Remove unnecessary semicolons drivers/isdn: Remove unnecessary semicolons drivers/leds: Remove unnecessary semicolons drivers/media/video: Remove unnecessary semicolons drivers/misc: Remove unnecessary semicolons drivers/mmc: Remove unnecessary semicolons drivers/net/bnx2x: Remove unnecessary semicolons drivers/net/e1000e: Remove unnecessary semicolons drivers/net/ixgbe: Remove unnecessary semicolons drivers/net/vxge: Remove unnecessary semicolons drivers/net/wireless/ath: Remove unnecessary semicolons drivers/net/wireless/iwlwifi: Remove unnecessary semicolons drivers/net/cnic.c: Remove unnecessary semicolons drivers/platform/x86: Remove unnecessary semicolons drivers/power: Remove unnecessary semicolons drivers/s390/net: Remove unnecessary semicolons drivers/scsi/be2iscsi: Remove unnecessary semicolons drivers/scsi/bfa: Remove unnecessary semicolons drivers/scsi/lpfc: Remove unnecessary semicolons drivers/scsi/pm8001: Remove unnecessary semicolons drivers/scsi/qla2xxx: Remove unnecessary semicolons drivers/serial: Remove unnecessary semicolons drivers/spi: Remove unnecessary semicolons drivers/staging: Remove unnecessary semicolons drivers/usb/gadget: Remove unnecessary semicolons drivers/xen: Remove unnecessary semicolons fs/9p: Remove unnecessary semicolons fs/ceph: Remove unnecessary semicolons fs/logfs: Remove unnecessary semicolons fs/nfs: Remove unnecessary semicolons fs/ocfs2: Remove unnecessary semicolons fs/ubifs: Remove unnecessary semicolons include/linux/if_macvlan.h: Remove unnecessary semicolons include/net/caif/cfctrl.h: Remove unnecessary semicolons mm/hugetlb.c: Remove unnecessary semicolons net/ipv6/mcast.c: Remove unnecessary semicolons net/sunrpc/addr.c: Remove unnecessary semicolons sound/core/pcm_lib.c: Remove unnecessary semicolons sound/soc/codecs: Remove unnecessary semicolons
arch/arm/mach-at91/at91cap9_devices.c | 2 +- arch/arm/mach-at91/at91sam9g45_devices.c | 2 +- arch/arm/mach-at91/at91sam9rl_devices.c | 2 +- arch/arm/mach-nuc93x/time.c | 2 +- arch/arm/mach-tegra/tegra2_clocks.c | 2 +- arch/arm/mach-w90x900/cpu.c | 2 +- arch/arm/plat-mxc/irq.c | 2 +- arch/microblaze/lib/memmove.c | 2 +- arch/um/drivers/mmapper_kern.c | 2 +- drivers/cpufreq/cpufreq_conservative.c | 2 +- drivers/gpio/langwell_gpio.c | 2 +- drivers/i2c/busses/i2c-designware.c | 2 +- drivers/isdn/hardware/mISDN/mISDNinfineon.c | 4 ++-- drivers/isdn/hardware/mISDN/mISDNisar.c | 2 +- drivers/leds/leds-mc13783.c | 2 +- drivers/media/video/cx88/cx88-blackbird.c | 2 +- drivers/media/video/davinci/vpfe_capture.c | 2 +- drivers/media/video/em28xx/em28xx-cards.c | 2 +- drivers/misc/bmp085.c | 2 +- drivers/misc/isl29020.c | 2 +- drivers/mmc/host/davinci_mmc.c | 2 +- drivers/net/bnx2x/bnx2x_link.c | 4 ++-- drivers/net/bnx2x/bnx2x_main.c | 2 +- drivers/net/cnic.c | 2 +- drivers/net/e1000e/netdev.c | 2 +- drivers/net/ixgbe/ixgbe_sriov.c | 2 +- drivers/net/vxge/vxge-main.c | 2 +- drivers/net/wireless/ath/ath9k/htc.h | 2 +- drivers/net/wireless/iwlwifi/iwl-agn.c | 2 +- drivers/platform/x86/classmate-laptop.c | 2 +- drivers/platform/x86/thinkpad_acpi.c | 2 +- drivers/power/intel_mid_battery.c | 2 +- drivers/s390/net/qeth_core_sys.c | 2 +- drivers/scsi/be2iscsi/be_main.c | 4 ++-- drivers/scsi/bfa/bfa_fcs_lport.c | 2 +- drivers/scsi/lpfc/lpfc_bsg.c | 2 +- drivers/scsi/pm8001/pm8001_init.c | 2 +- drivers/scsi/qla2xxx/qla_isr.c | 4 ++-- drivers/scsi/qla2xxx/qla_nx.c | 2 +- drivers/serial/mrst_max3110.c | 2 +- drivers/spi/amba-pl022.c | 2 +- drivers/spi/spi_nuc900.c | 2 +- .../staging/ath6kl/hif/sdio/linux_sdio/src/hif.c | 2 +- drivers/staging/ath6kl/os/linux/ar6000_drv.c | 2 +- drivers/staging/bcm/InterfaceInit.c | 2 +- drivers/staging/bcm/InterfaceIsr.c | 2 +- drivers/staging/bcm/Misc.c | 4 ++-- .../comedi/drivers/addi-data/APCI1710_Tor.c | 2 +- .../comedi/drivers/addi-data/hwdrv_apci1500.c | 2 +- .../comedi/drivers/addi-data/hwdrv_apci1516.c | 2 +- .../comedi/drivers/addi-data/hwdrv_apci3501.c | 2 +- drivers/staging/comedi/drivers/amplc_pci230.c | 2 +- drivers/staging/comedi/drivers/cb_das16_cs.c | 2 +- drivers/staging/comedi/drivers/comedi_bond.c | 2 +- drivers/staging/crystalhd/crystalhd_hw.c | 2 +- drivers/staging/go7007/go7007-driver.c | 2 +- drivers/staging/iio/accel/lis3l02dq_ring.c | 2 +- .../staging/intel_sst/intel_sst_drv_interface.c | 4 ++-- drivers/staging/keucr/smilmain.c | 4 ++-- drivers/staging/keucr/smilsub.c | 4 ++-- drivers/staging/msm/lcdc_toshiba_wvga_pt.c | 2 +- drivers/staging/rt2860/common/cmm_data_pci.c | 4 ++-- drivers/staging/rt2860/rt_linux.c | 2 +- drivers/staging/rt2860/rtmp.h | 2 +- drivers/staging/rtl8192e/ieee80211/ieee80211_tx.c | 2 +- drivers/staging/rtl8192e/r819xE_phy.c | 2 +- drivers/staging/rtl8192u/ieee80211/ieee80211_tx.c | 2 +- drivers/staging/rtl8192u/r8192U_core.c | 2 +- drivers/staging/rtl8192u/r819xU_phy.c | 2 +- drivers/staging/rtl8712/rtl8712_efuse.c | 2 +- drivers/staging/rtl8712/rtl8712_xmit.c | 2 +- drivers/staging/rtl8712/rtl871x_xmit.c | 2 +- drivers/staging/tidspbridge/core/tiomap3430.c | 4 ++-- drivers/staging/tidspbridge/rmgr/nldr.c | 2 +- drivers/staging/vt6655/card.c | 2 +- drivers/staging/vt6655/iwctl.c | 2 +- drivers/staging/vt6655/wpa2.c | 4 ++-- drivers/staging/vt6656/baseband.c | 2 +- drivers/staging/vt6656/iwctl.c | 2 +- drivers/staging/vt6656/power.c | 2 +- drivers/staging/vt6656/wpa2.c | 4 ++-- drivers/usb/gadget/f_fs.c | 2 +- drivers/xen/swiotlb-xen.c | 2 +- fs/9p/acl.c | 2 +- fs/9p/xattr.c | 2 +- fs/ceph/mds_client.c | 2 +- fs/logfs/readwrite.c | 2 +- fs/nfs/getroot.c | 2 +- fs/ocfs2/refcounttree.c | 2 +- fs/ubifs/scan.c | 2 +- include/linux/if_macvlan.h | 2 +- include/net/caif/cfctrl.h | 2 +- mm/hugetlb.c | 2 +- net/ipv6/mcast.c | 2 +- net/sunrpc/addr.c | 2 +- sound/core/pcm_lib.c | 2 +- sound/soc/codecs/wm8904.c | 2 +- sound/soc/codecs/wm8940.c | 1 - sound/soc/codecs/wm8993.c | 2 +- sound/soc/codecs/wm_hubs.c | 2 +- 100 files changed, 111 insertions(+), 112 deletions(-)
Signed-off-by: Joe Perches joe@perches.com --- sound/core/pcm_lib.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/sound/core/pcm_lib.c b/sound/core/pcm_lib.c index a1707cc..b75db8e 100644 --- a/sound/core/pcm_lib.c +++ b/sound/core/pcm_lib.c @@ -223,7 +223,7 @@ static void xrun_log(struct snd_pcm_substream *substream, entry->jiffies = jiffies; entry->pos = pos; entry->period_size = runtime->period_size; - entry->buffer_size = runtime->buffer_size;; + entry->buffer_size = runtime->buffer_size; entry->old_hw_ptr = runtime->status->hw_ptr; entry->hw_ptr_base = runtime->hw_ptr_base; log->idx = (log->idx + 1) % XRUN_LOG_CNT;
At Sun, 14 Nov 2010 19:05:02 -0800, Joe Perches wrote:
Signed-off-by: Joe Perches joe@perches.com
Applied now. Thanks.
Takashi
sound/core/pcm_lib.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/sound/core/pcm_lib.c b/sound/core/pcm_lib.c index a1707cc..b75db8e 100644 --- a/sound/core/pcm_lib.c +++ b/sound/core/pcm_lib.c @@ -223,7 +223,7 @@ static void xrun_log(struct snd_pcm_substream *substream, entry->jiffies = jiffies; entry->pos = pos; entry->period_size = runtime->period_size;
- entry->buffer_size = runtime->buffer_size;;
- entry->buffer_size = runtime->buffer_size; entry->old_hw_ptr = runtime->status->hw_ptr; entry->hw_ptr_base = runtime->hw_ptr_base; log->idx = (log->idx + 1) % XRUN_LOG_CNT;
-- 1.7.3.1.g432b3.dirty
Signed-off-by: Joe Perches joe@perches.com --- sound/soc/codecs/wm8904.c | 2 +- sound/soc/codecs/wm8940.c | 1 - sound/soc/codecs/wm8993.c | 2 +- sound/soc/codecs/wm_hubs.c | 2 +- 4 files changed, 3 insertions(+), 4 deletions(-)
diff --git a/sound/soc/codecs/wm8904.c b/sound/soc/codecs/wm8904.c index 33be84e..99ae66d 100644 --- a/sound/soc/codecs/wm8904.c +++ b/sound/soc/codecs/wm8904.c @@ -1590,7 +1590,7 @@ static int wm8904_hw_params(struct snd_pcm_substream *substream, - wm8904->fs); for (i = 1; i < ARRAY_SIZE(clk_sys_rates); i++) { cur_val = abs((wm8904->sysclk_rate / - clk_sys_rates[i].ratio) - wm8904->fs);; + clk_sys_rates[i].ratio) - wm8904->fs); if (cur_val < best_val) { best = i; best_val = cur_val; diff --git a/sound/soc/codecs/wm8940.c b/sound/soc/codecs/wm8940.c index 2cb16f8..e3f3572 100644 --- a/sound/soc/codecs/wm8940.c +++ b/sound/soc/codecs/wm8940.c @@ -735,7 +735,6 @@ static int wm8940_probe(struct snd_soc_codec *codec) return ret;
return ret; -; }
static int wm8940_remove(struct snd_soc_codec *codec) diff --git a/sound/soc/codecs/wm8993.c b/sound/soc/codecs/wm8993.c index 589e3fa..74af9c5 100644 --- a/sound/soc/codecs/wm8993.c +++ b/sound/soc/codecs/wm8993.c @@ -1225,7 +1225,7 @@ static int wm8993_hw_params(struct snd_pcm_substream *substream, - wm8993->fs); for (i = 1; i < ARRAY_SIZE(clk_sys_rates); i++) { cur_val = abs((wm8993->sysclk_rate / - clk_sys_rates[i].ratio) - wm8993->fs);; + clk_sys_rates[i].ratio) - wm8993->fs); if (cur_val < best_val) { best = i; best_val = cur_val; diff --git a/sound/soc/codecs/wm_hubs.c b/sound/soc/codecs/wm_hubs.c index 19ca782..2afbc7a 100644 --- a/sound/soc/codecs/wm_hubs.c +++ b/sound/soc/codecs/wm_hubs.c @@ -112,7 +112,7 @@ static void calibrate_dc_servo(struct snd_soc_codec *codec) switch (hubs->dcs_readback_mode) { case 0: reg_l = snd_soc_read(codec, WM8993_DC_SERVO_READBACK_1) - & WM8993_DCS_INTEG_CHAN_0_MASK;; + & WM8993_DCS_INTEG_CHAN_0_MASK; reg_r = snd_soc_read(codec, WM8993_DC_SERVO_READBACK_2) & WM8993_DCS_INTEG_CHAN_1_MASK; break;
On Sun, 2010-11-14 at 19:05 -0800, Joe Perches wrote:
Signed-off-by: Joe Perches joe@perches.com
sound/soc/codecs/wm8904.c | 2 +- sound/soc/codecs/wm8940.c | 1 - sound/soc/codecs/wm8993.c | 2 +- sound/soc/codecs/wm_hubs.c | 2 +- 4 files changed, 3 insertions(+), 4 deletions(-)
Acked-by: Liam Girdwood lrg@slimlogic.co.uk
On Sun, Nov 14, 2010 at 07:05:03PM -0800, Joe Perches wrote:
Signed-off-by: Joe Perches joe@perches.com
This doesn't apply against current -next, could you please regenerate against that?
Signed-off-by: Joe Perches joe@perches.com --- V2: against -next
sound/soc/codecs/wm8904.c | 2 +- sound/soc/codecs/wm8940.c | 1 - sound/soc/codecs/wm8993.c | 2 +- sound/soc/codecs/wm_hubs.c | 2 +- 4 files changed, 3 insertions(+), 4 deletions(-)
diff --git a/sound/soc/codecs/wm8904.c b/sound/soc/codecs/wm8904.c index be90399..5e57bd2 100644 --- a/sound/soc/codecs/wm8904.c +++ b/sound/soc/codecs/wm8904.c @@ -1591,7 +1591,7 @@ static int wm8904_hw_params(struct snd_pcm_substream *substream, - wm8904->fs); for (i = 1; i < ARRAY_SIZE(clk_sys_rates); i++) { cur_val = abs((wm8904->sysclk_rate / - clk_sys_rates[i].ratio) - wm8904->fs);; + clk_sys_rates[i].ratio) - wm8904->fs); if (cur_val < best_val) { best = i; best_val = cur_val; diff --git a/sound/soc/codecs/wm8940.c b/sound/soc/codecs/wm8940.c index c2def1b..caed084 100644 --- a/sound/soc/codecs/wm8940.c +++ b/sound/soc/codecs/wm8940.c @@ -736,7 +736,6 @@ static int wm8940_probe(struct snd_soc_codec *codec) return ret;
return ret; -; }
static int wm8940_remove(struct snd_soc_codec *codec) diff --git a/sound/soc/codecs/wm8993.c b/sound/soc/codecs/wm8993.c index bcc54be..991d90c 100644 --- a/sound/soc/codecs/wm8993.c +++ b/sound/soc/codecs/wm8993.c @@ -1227,7 +1227,7 @@ static int wm8993_hw_params(struct snd_pcm_substream *substream, - wm8993->fs); for (i = 1; i < ARRAY_SIZE(clk_sys_rates); i++) { cur_val = abs((wm8993->sysclk_rate / - clk_sys_rates[i].ratio) - wm8993->fs);; + clk_sys_rates[i].ratio) - wm8993->fs); if (cur_val < best_val) { best = i; best_val = cur_val; diff --git a/sound/soc/codecs/wm_hubs.c b/sound/soc/codecs/wm_hubs.c index 8aff0ef..422c7fb 100644 --- a/sound/soc/codecs/wm_hubs.c +++ b/sound/soc/codecs/wm_hubs.c @@ -119,7 +119,7 @@ static void calibrate_dc_servo(struct snd_soc_codec *codec) switch (hubs->dcs_readback_mode) { case 0: reg_l = snd_soc_read(codec, WM8993_DC_SERVO_READBACK_1) - & WM8993_DCS_INTEG_CHAN_0_MASK;; + & WM8993_DCS_INTEG_CHAN_0_MASK; reg_r = snd_soc_read(codec, WM8993_DC_SERVO_READBACK_2) & WM8993_DCS_INTEG_CHAN_1_MASK; break;
On Mon, Nov 15, 2010 at 09:09:17AM -0800, Joe Perches wrote:
Signed-off-by: Joe Perches joe@perches.com
Applied, thanks.
Please try to use changelog formats consistent with the code you're modifying.
On Mon, 2010-11-15 at 17:30 +0000, Mark Brown wrote:
On Mon, Nov 15, 2010 at 09:09:17AM -0800, Joe Perches wrote:
Signed-off-by: Joe Perches joe@perches.com
Applied, thanks. Please try to use changelog formats consistent with the code you're modifying.
I think it's more important to use consistent changelogs for a patch series.
If you want your own subsystem changelog consistency, I think you should change the format to what you desire.
cheers, Joe
On Mon, Nov 15, 2010 at 09:34:04AM -0800, Joe Perches wrote:
On Mon, 2010-11-15 at 17:30 +0000, Mark Brown wrote:
Please try to use changelog formats consistent with the code you're modifying.
I think it's more important to use consistent changelogs for a patch series.
...since...?
If you want your own subsystem changelog consistency, I think you should change the format to what you desire.
Which is what I'm doing but it's annoying to have to constantly hand edit changelogs.
On Mon, 2010-11-15 at 18:27 +0000, Mark Brown wrote:
On Mon, Nov 15, 2010 at 09:34:04AM -0800, Joe Perches wrote:
On Mon, 2010-11-15 at 17:30 +0000, Mark Brown wrote:
Please try to use changelog formats consistent with the code you're modifying.
I think it's more important to use consistent changelogs for a patch series.
...since...?
1995...
Since there isn't a consistent standard for subsystems changelogs and automating scripts for the desires of individual subsystem maintainers is not feasible.
On Mon, Nov 15, 2010 at 10:30:29AM -0800, Joe Perches wrote:
On Mon, 2010-11-15 at 18:27 +0000, Mark Brown wrote:
On Mon, Nov 15, 2010 at 09:34:04AM -0800, Joe Perches wrote:
I think it's more important to use consistent changelogs for a patch series.
...since...?
1995...
That's not really a reason. It seems that...
Since there isn't a consistent standard for subsystems changelogs and automating scripts for the desires of individual subsystem maintainers is not feasible.
...you mean that you wish to do this since it makes your life as a script author easier. I'd suggest using pattern matching to look up the rules for generating the prefixes (it's pretty much entirely prefixes) in the same way you're handling figuring out who to mail - that'd probably cover it in an automatable fashion.
On Mon, 2010-11-15 at 19:07 +0000, Mark Brown wrote:
I'd suggest using pattern matching to look up the rules for generating the prefixes (it's pretty much entirely prefixes) in the same way you're handling figuring out who to mail - that'd probably cover it in an automatable fashion.
Publish a tool that works and I'll use it.
On Mon, Nov 15, 2010 at 11:14:18AM -0800, Joe Perches wrote:
On Mon, 2010-11-15 at 19:07 +0000, Mark Brown wrote:
I'd suggest using pattern matching to look up the rules for generating the prefixes (it's pretty much entirely prefixes) in the same way you're handling figuring out who to mail - that'd probably cover it in an automatable fashion.
Publish a tool that works and I'll use it.
It appears your scripts are already hooked into get_maintainers.pl which would seem the obvious place to do this? Sadly I don't do perl, though it looks like you're doing pretty much all the work on that anyway.
The main thing here is to avoid your patches sticking out - as well as the hassle applying them stuff like this is also a red flag on review.
On Mon, 2010-11-15 at 19:34 +0000, Mark Brown wrote:
On Mon, Nov 15, 2010 at 11:14:18AM -0800, Joe Perches wrote:
On Mon, 2010-11-15 at 19:07 +0000, Mark Brown wrote:
I'd suggest using pattern matching to look up the rules for generating the prefixes (it's pretty much entirely prefixes) in the same way you're handling figuring out who to mail - that'd probably cover it in an automatable fashion.
Publish a tool that works and I'll use it.
It appears your scripts are already hooked into get_maintainers.pl which would seem the obvious place to do this? Sadly I don't do perl, though it looks like you're doing pretty much all the work on that anyway.
Sadly, no it's not the right place.
That script just generates cc email addresses for pre-formatted commit patches.
It'd have to be a script that modifies the git commit subject line to the taste of the subsystem maintainer.
Right now, I use a commit script that's something like:
#!/bin/bash echo "$1: Remove unnecessary semicolons" > msg echo >> msg #cat >> msg <<EOF #Unnecessary semicolons should not exist. #EOF git commit -s -F msg $1
There could be a modification to $1 (path) or some such.
Maybe a script like ./scripts/convert_commit_subject_to_subsystem_maintainer_taste or something.
Care to write one in sh/bash/perl/python/c/ocaml/c#?
As far as I know, the only subsystem pedants^H^H^H^H^Hople that care much about the commit subject style are arch/x86 and sound.
I can understand the desire of these subsystem maintainers to have a consistent style. I think though that requiring a subject header style without providing more than a general guideline is a but much.
I'd use any other automated tool you want to provide.
cheers, Joe
On Mon, Nov 15, 2010 at 11:52:53AM -0800, Joe Perches wrote:
On Mon, 2010-11-15 at 19:34 +0000, Mark Brown wrote:
It appears your scripts are already hooked into get_maintainers.pl which would seem the obvious place to do this? Sadly I don't do perl, though it looks like you're doing pretty much all the work on that anyway.
Sadly, no it's not the right place.
To query MAINTAINERS? I'd assume that's where you'd want to put that stuff?
There could be a modification to $1 (path) or some such.
Maybe a script like ./scripts/convert_commit_subject_to_subsystem_maintainer_taste or something.
Care to write one in sh/bash/perl/python/c/ocaml/c#?
Like I say I'd expect this to be a get_maintainers based lookup to dump some data out?
As far as I know, the only subsystem pedants^H^H^H^H^Hople that care much about the commit subject style are arch/x86 and sound.
If you look at the kernel you'll see quite a few subsystems which have some sort of standard practice which they do try to enforce, you shouldn't take silence as people being happy here - it's taken me some considerable time to get round to mentioning this, for example, and I might not have bothered if the patch had applied first time around. Like working against -next it's one of these things that would make your patches easier to deal with.
I can understand the desire of these subsystem maintainers to have a consistent style. I think though that requiring a subject header style without providing more than a general guideline is a but much.
The general guideline I tend to go with is that if what you're doing looks odd for the code you're submitting against for some reason you're doing something wrong unless you understand why you're doing something different and there's a good reason.
I'd use any other automated tool you want to provide.
Like I say, I'd expect the lookup from the database to be handled by get_maintainers.pl. Having a separate database would seem odd.
On Tue, 2010-11-16 at 10:49 +0000, Mark Brown wrote:
On Mon, Nov 15, 2010 at 11:52:53AM -0800, Joe Perches wrote:
On Mon, 2010-11-15 at 19:34 +0000, Mark Brown wrote:
It appears your scripts are already hooked into get_maintainers.pl which would seem the obvious place to do this? Sadly I don't do perl, though it looks like you're doing pretty much all the work on that anyway.
Sadly, no it's not the right place.
To query MAINTAINERS? I'd assume that's where you'd want to put that stuff?
I trimmed cc's and added Andrew Morton and Florian Mickler. First thread link for them: http://lkml.org/lkml/2010/11/15/262
I use get_maintainer to find email addresses with "git send-email --cc-cmd=" but sure it could be extended to find some other new information in the MAINTAINERS file.
Anyway, I think that get_maintainers isn't the proper tool to rewrite commit subject lines, though it could certainly do the lookup of a key in the MAINTAINERS file.
Maybe add a new MAINTAINERS section line something like: "C: CommitSubjectGrammarStyle" where CommitSubjectGrammarStyle is something more information rich than "style 1", "style 2".
Perhaps you'll propose a grammar to convert path to header and go through and add these "C:" style entries to the sections you maintain.
Also, what would you expect the output to be when a single patch modified files from 2 subsystems that use different styles?
cheers, Joe
On Tue, Nov 16, 2010 at 06:51:17AM -0800, Joe Perches wrote:
Maybe add a new MAINTAINERS section line something like: "C: CommitSubjectGrammarStyle" where CommitSubjectGrammarStyle is something more information rich than "style 1", "style 2".
Something printfish would seem reasonable?
Also, what would you expect the output to be when a single patch modified files from 2 subsystems that use different styles?
The traditional thing is "ThingX/ThingY: blah" but as with anything else you need to be sensible.
On Tue, 16 Nov 2010 06:51:17 -0800 Joe Perches joe@perches.com wrote:
On Tue, 2010-11-16 at 10:49 +0000, Mark Brown wrote:
On Mon, Nov 15, 2010 at 11:52:53AM -0800, Joe Perches wrote:
On Mon, 2010-11-15 at 19:34 +0000, Mark Brown wrote:
It appears your scripts are already hooked into get_maintainers.pl which would seem the obvious place to do this? Sadly I don't do perl, though it looks like you're doing pretty much all the work on that anyway.
Sadly, no it's not the right place.
To query MAINTAINERS? I'd assume that's where you'd want to put that stuff?
I trimmed cc's and added Andrew Morton and Florian Mickler. First thread link for them: http://lkml.org/lkml/2010/11/15/262
I use get_maintainer to find email addresses with "git send-email --cc-cmd=" but sure it could be extended to find some other new information in the MAINTAINERS file.
Anyway, I think that get_maintainers isn't the proper tool to rewrite commit subject lines, though it could certainly do the lookup of a key in the MAINTAINERS file.
Maybe add a new MAINTAINERS section line something like: "C: CommitSubjectGrammarStyle" where CommitSubjectGrammarStyle is something more information rich than "style 1", "style 2".
Perhaps you'll propose a grammar to convert path to header and go through and add these "C:" style entries to the sections you maintain.
Also, what would you expect the output to be when a single patch modified files from 2 subsystems that use different styles?
cheers, Joe
My first reaction to this is, it's silly. Certainly a subsystem-maintainer is capable of hacking something together that suits his needs or may just use a good editor to get the job done. After all, he might want to edit the commit message anyway. Also he has to have his act together for all non-conforming submitters anyway, because shurely, telling people to re-edit their patches subject line is not what one would consider "welcoming to newbies", or whatever it is kernel subsystem maintainers have to be nowadays *g*...
On second thought, if that facility existed, i think nobody would mind it either. So, why not. I don't see a way to specify what to do with cross-subsystem patches though.
(MAINTAINERS seems to be the logical place to put this information.)
Regards, Flo
On Tue, Nov 16, 2010 at 06:37:07PM +0100, Florian Mickler wrote:
My first reaction to this is, it's silly. Certainly a subsystem-maintainer is capable of hacking something together that suits his needs or may just use a good editor to get the job done. After all, he might want to edit the commit message anyway. Also he has to have his act together for all non-conforming submitters anyway, because shurely, telling people to re-edit their patches subject line is not what one would consider "welcoming to newbies", or whatever it is kernel subsystem maintainers have to be nowadays *g*...
So, my general policy on this is that I tend to push back on patches which don't just work with the toolset (subject lines are just one part of it) to a variable extent depending on who's submitting and what they're submitting. One of the factors is that the more patches are coming from someone the easier I expect their patches to be to work with.
The reason this came up is that this is one of the issues with Joe's patches (which are rather frequent) but he is only willing to do things that he can automate.
(MAINTAINERS seems to be the logical place to put this information.)
Indeed.
On Tue, 16 Nov 2010 18:12:27 +0000 Mark Brown broonie@opensource.wolfsonmicro.com wrote:
On Tue, Nov 16, 2010 at 06:37:07PM +0100, Florian Mickler wrote:
My first reaction to this is, it's silly. Certainly a subsystem-maintainer is capable of hacking something together that suits his needs or may just use a good editor to get the job done. After all, he might want to edit the commit message anyway. Also he has to have his act together for all non-conforming submitters anyway, because shurely, telling people to re-edit their patches subject line is not what one would consider "welcoming to newbies", or whatever it is kernel subsystem maintainers have to be nowadays *g*...
So, my general policy on this is that I tend to push back on patches which don't just work with the toolset (subject lines are just one part of it) to a variable extent depending on who's submitting and what they're submitting. One of the factors is that the more patches are coming from someone the easier I expect their patches to be to work with.
The reason this came up is that this is one of the issues with Joe's patches (which are rather frequent) but he is only willing to do things that he can automate.
Hehe, I know that I wouldn't want to hand edit every autogenerated patch people throw at me... What about just dropping everything before the last "]" or ":" and putting an autogenerated prefix before it in a pre-commit hook on your side?
That should work most of the time... don't know... maybe other subsystem maintainers have some more suggestions on reducing the workload... this could even be an interesting topic for some summit...
Regards, Flo
On Tue, Nov 16, 2010 at 08:35:22PM +0100, Florian Mickler wrote:
Hehe, I know that I wouldn't want to hand edit every autogenerated patch people throw at me... What about just dropping everything before the last "]" or ":" and putting an autogenerated prefix before it in a pre-commit hook on your side?
That should work most of the time... don't know... maybe other
It's the most of the time bit that worries me, I'm generally reluctant to script things like this when the scripts aren't very widely used and it's a pain to get hooks distributed over all my systems and working for all the things I need to apply patches for.
_From my point of view my current approach is actually working pretty well with most submitters, even people doing similar janitorial stuff.
On Tue, 16 Nov 2010 19:55:31 +0000 Mark Brown wrote:
On Tue, Nov 16, 2010 at 08:35:22PM +0100, Florian Mickler wrote:
Hehe, I know that I wouldn't want to hand edit every autogenerated patch people throw at me... What about just dropping everything before the last "]" or ":" and putting an autogenerated prefix before it in a pre-commit hook on your side?
That should work most of the time... don't know... maybe other
It's the most of the time bit that worries me, I'm generally reluctant to script things like this when the scripts aren't very widely used and it's a pain to get hooks distributed over all my systems and working for all the things I need to apply patches for.
From my point of view my current approach is actually working pretty well with most submitters, even people doing similar janitorial stuff.
I don't know what you asked Joe to change, but asking someone to use the documented canonical patch format:
<quote> The canonical patch subject line is:
Subject: [PATCH 001/123] subsystem: summary phrase </quote>
should be fine. And there is no need for printf-ish templates for this in MAINTAINERS either.
--- ~Randy *** Remember to use Documentation/SubmitChecklist when testing your code ***
On Tue, 2010-11-16 at 12:21 -0800, Randy Dunlap wrote:
On Tue, 16 Nov 2010 19:55:31 +0000 Mark Brown wrote: I don't know what you asked Joe to change, but asking someone to use the documented canonical patch format:
<quote> The canonical patch subject line is: Subject: [PATCH 001/123] subsystem: summary phrase </quote> should be fine. And there is no need for printf-ish templates for this in MAINTAINERS either.
I've never read that before. Learn something new etc... It seems path prefixes aren't good nor even commonly used.
A review of kernel patch subjects:
$ git log --no-merges --pretty=oneline | \ cut -f2- -d" " | cut -f1 -d: | sort | uniq -c | sort -rn
is interesting. Here's the head: 5007 x86 3943 Staging 3220 USB 2790 sh 2707 KVM 2624 ARM 2449 ALSA 1571 Input 1549 ASoC 1470 iwlwifi 1423 ACPI 1397 mac80211 1384 V4L/DVB 1226 sched 1200 Btrfs 1184 powerpc 1106 [NETFILTER] 1080 MIPS 1049 net 1047 ide 1014 drm/i915 993 staging 921 ath9k
Some subsystem maintainers like upper case, some mixed, some lower. Some aren't consistent. (Staging/staging)
It doesn't seem a rule can be pregenerated so maybe adding these "C:" lines to MAINTAINERS has some value.
On Tue, 16 Nov 2010 12:42:36 -0800 Joe Perches wrote:
On Tue, 2010-11-16 at 12:21 -0800, Randy Dunlap wrote:
On Tue, 16 Nov 2010 19:55:31 +0000 Mark Brown wrote: I don't know what you asked Joe to change, but asking someone to use the documented canonical patch format:
<quote> The canonical patch subject line is: Subject: [PATCH 001/123] subsystem: summary phrase </quote> should be fine. And there is no need for printf-ish templates for this in MAINTAINERS either.
I've never read that before. Learn something new etc... It seems path prefixes aren't good nor even commonly used.
A review of kernel patch subjects:
$ git log --no-merges --pretty=oneline | \ cut -f2- -d" " | cut -f1 -d: | sort | uniq -c | sort -rn
is interesting. Here's the head: 5007 x86 3943 Staging 3220 USB 2790 sh 2707 KVM 2624 ARM 2449 ALSA 1571 Input 1549 ASoC 1470 iwlwifi 1423 ACPI 1397 mac80211 1384 V4L/DVB 1226 sched 1200 Btrfs 1184 powerpc 1106 [NETFILTER] 1080 MIPS 1049 net 1047 ide 1014 drm/i915 993 staging 921 ath9k
$ARCH is a commonly accepted substitute for subsystem.
And yes, lots of people use <drivername> there as well.
Some subsystem maintainers like upper case, some mixed, some lower. Some aren't consistent. (Staging/staging)
Case usually doesn't matter to most of us.
It doesn't seem a rule can be pregenerated so maybe adding these "C:" lines to MAINTAINERS has some value.
Hopefully it won't go that far.
--- ~Randy *** Remember to use Documentation/SubmitChecklist when testing your code ***
On Tue, Nov 16, 2010 at 12:46:09PM -0800, Randy Dunlap wrote:
On Tue, 16 Nov 2010 12:42:36 -0800 Joe Perches wrote:
Some subsystem maintainers like upper case, some mixed, some lower. Some aren't consistent. (Staging/staging)
Case usually doesn't matter to most of us.
Given that we're working in case sensitive languages here it's probably safe to assume that a reasonable proportion of people will care; being reasonably consistent with existing practice for the subsystem seems sensible.
On Tue, 16 Nov 2010 23:22:58 +0000 Mark Brown wrote:
On Tue, Nov 16, 2010 at 12:46:09PM -0800, Randy Dunlap wrote:
On Tue, 16 Nov 2010 12:42:36 -0800 Joe Perches wrote:
Some subsystem maintainers like upper case, some mixed, some lower. Some aren't consistent. (Staging/staging)
Case usually doesn't matter to most of us.
Given that we're working in case sensitive languages here it's probably safe to assume that a reasonable proportion of people will care; being reasonably consistent with existing practice for the subsystem seems sensible.
Greg takes patches that say STAGING or Staging or staging.
DaveM takes patches that say net: or netdev: or network: or NET:
The sound maintainers take patches that say sound: or alsa: or ALSA:
etc.
--- ~Randy *** Remember to use Documentation/SubmitChecklist when testing your code ***
On Tue, Nov 16, 2010 at 03:28:35PM -0800, Randy Dunlap wrote:
On Tue, 16 Nov 2010 23:22:58 +0000 Mark Brown wrote:
On Tue, Nov 16, 2010 at 12:46:09PM -0800, Randy Dunlap wrote:
Case usually doesn't matter to most of us.
Given that we're working in case sensitive languages here it's probably safe to assume that a reasonable proportion of people will care; being reasonably consistent with existing practice for the subsystem seems sensible.
Greg takes patches that say STAGING or Staging or staging.
DaveM takes patches that say net: or netdev: or network: or NET:
The sound maintainers take patches that say sound: or alsa: or ALSA:
etc.
...and best practice would be to pay attention to what the standard thing is for the subsystem and follow that. We shouldn't be suggesting that people just ignore the case, though obviously if it's not clear then it's not worth worrying too much about it.
On Tue, 2010-11-16 at 15:28 -0800, Randy Dunlap wrote:
On Tue, 16 Nov 2010 23:22:58 +0000 Mark Brown wrote:
On Tue, Nov 16, 2010 at 12:46:09PM -0800, Randy Dunlap wrote:
On Tue, 16 Nov 2010 12:42:36 -0800 Joe Perches wrote:
Some subsystem maintainers like upper case, some mixed, some lower. Some aren't consistent. (Staging/staging)
Case usually doesn't matter to most of us.
Given that we're working in case sensitive languages here it's probably safe to assume that a reasonable proportion of people will care; being reasonably consistent with existing practice for the subsystem seems sensible.
Greg takes patches that say STAGING or Staging or staging.
Greg seems to rewrite patch subjects and is inconsistent about case, so he might be doing that by hand.
DaveM takes patches that say net: or netdev: or network: or NET:
DaveM doesn't appear to be choosy about patch subject lines. He seems to take any sensible patch and as far as I know he doesn't edit the subject lines. He does reject inferior patches outright.
The sound maintainers take patches that say sound: or alsa: or ALSA:
The sound maintainers seem to rewrite patch subjects on an ad-hoc basis.
On Tue, 16 Nov 2010 15:57:55 -0800 Joe Perches wrote:
On Tue, 2010-11-16 at 15:28 -0800, Randy Dunlap wrote:
On Tue, 16 Nov 2010 23:22:58 +0000 Mark Brown wrote:
On Tue, Nov 16, 2010 at 12:46:09PM -0800, Randy Dunlap wrote:
On Tue, 16 Nov 2010 12:42:36 -0800 Joe Perches wrote:
Some subsystem maintainers like upper case, some mixed, some lower. Some aren't consistent. (Staging/staging)
Case usually doesn't matter to most of us.
Given that we're working in case sensitive languages here it's probably safe to assume that a reasonable proportion of people will care; being reasonably consistent with existing practice for the subsystem seems sensible.
Greg takes patches that say STAGING or Staging or staging.
Greg seems to rewrite patch subjects and is inconsistent about case, so he might be doing that by hand.
DaveM takes patches that say net: or netdev: or network: or NET:
DaveM doesn't appear to be choosy about patch subject lines. He seems to take any sensible patch and as far as I know he doesn't edit the subject lines. He does reject inferior patches outright.
The sound maintainers take patches that say sound: or alsa: or ALSA:
The sound maintainers seem to rewrite patch subjects on an ad-hoc basis.
OK, I can accept your summary. However, I can't tell that we are making any progress.
--- ~Randy *** Remember to use Documentation/SubmitChecklist when testing your code ***
On Tue, 2010-11-16 at 23:22 +0000, Mark Brown wrote:
On Tue, Nov 16, 2010 at 12:46:09PM -0800, Randy Dunlap wrote:
On Tue, 16 Nov 2010 12:42:36 -0800 Joe Perches wrote:
Some subsystem maintainers like upper case, some mixed, some lower. Some aren't consistent. (Staging/staging)
Case usually doesn't matter to most of us.
Given that we're working in case sensitive languages here it's probably safe to assume that a reasonable proportion of people will care; being reasonably consistent with existing practice for the subsystem seems sensible.
Presumably the tool would also have to traverse up the tree to find the appropriate style so every MAINTAINERS section would not need a C entry.
ie: sound/soc/codecs/foo could use the C: entry for sound/soc/
Perhaps something like: C: ASoC basename:
and for arch/x86/: C: x86, dirname:
etc.
On Tue, Nov 16, 2010 at 12:21:02PM -0800, Randy Dunlap wrote:
I don't know what you asked Joe to change, but asking someone to use the documented canonical patch format:
<quote> The canonical patch subject line is:
Subject: [PATCH 001/123] subsystem: summary phrase</quote>
should be fine. And there is no need for printf-ish templates for this in MAINTAINERS either.
That's exactly what I asked him to do. He said he's not willing to use anything for "subsystem" which can't be automatically generated.
The formats I mentioned because some subsystems have their own things within this format like "subsystem: driver:" or whatever. While it's probably not an issue for the sort of patch Joe generates if we do have a tool for this I'd expect it'll go the same way that checkpatch does and get used by people doing more specific work. It'd be good to try to head off the friction that may cause by at least having an idea how we might cope with that.
On Nov 16 Mark Brown wrote:
On Tue, Nov 16, 2010 at 12:21:02PM -0800, Randy Dunlap wrote:
I don't know what you asked Joe to change, but asking someone to use the documented canonical patch format:
<quote> The canonical patch subject line is:
Subject: [PATCH 001/123] subsystem: summary phrase</quote>
should be fine. And there is no need for printf-ish templates for this in MAINTAINERS either.
That's exactly what I asked him to do. He said he's not willing to use anything for "subsystem" which can't be automatically generated.
Why should we codify our conventions in MAINTAINERS to accommodate the specific problem of virtually a _single_ patch author?
Conventions are living and are being adjusted all the time, as code organization changes, people join and go, projects start and cease.
Said author please looks the conventions up in the git history. If he finds that this decelerates his patch generation rate, he can surely code a script that looks into git for him and suggests plausible prefixes for his patch titles to him. Or he can collect a kind of database (a config file) locally for his own use in which he records conventional prefixes on the go.
On Wed, 17 Nov 2010, Stefan Richter wrote:
I don't know what you asked Joe to change, but asking someone to use the documented canonical patch format:
<quote> The canonical patch subject line is:
Subject: [PATCH 001/123] subsystem: summary phrase</quote>
should be fine. And there is no need for printf-ish templates for this in MAINTAINERS either.
That's exactly what I asked him to do. He said he's not willing to use anything for "subsystem" which can't be automatically generated.
Why should we codify our conventions in MAINTAINERS to accommodate the specific problem of virtually a _single_ patch author?
Conventions are living and are being adjusted all the time, as code organization changes, people join and go, projects start and cease.
Said author please looks the conventions up in the git history. If he finds that this decelerates his patch generation rate, he can surely code a script that looks into git for him and suggests plausible prefixes for his patch titles to him. Or he can collect a kind of database (a config file) locally for his own use in which he records conventional prefixes on the go.
Come on guys, this debate is really horribly boring.
Either the maintainer wants the patch. Then he is certainly able to apply it no matter the subject line (I personally am getting a lot of patches which don't follow the format I am using in my tree ... converting Subject: lines is so trivial that I have never felt like bothering anyone about it ... it's basically single condition in a shellscript). Or the maintainer doesn't feel like the patch is worth it, and then the subject-line format really doesn't matter.
On Wed, Nov 17, 2010 at 01:53:35AM +0100, Jiri Kosina wrote:
On Wed, 17 Nov 2010, Stefan Richter wrote:
Why should we codify our conventions in MAINTAINERS to accommodate the specific problem of virtually a _single_ patch author?
It seems to be the way we're heading in general - look at all the recent work on MAINTAINERS and get_maintainer.pl. There seems to be a genral push to make all this stuff automatable.
Either the maintainer wants the patch. Then he is certainly able to apply it no matter the subject line (I personally am getting a lot of patches which don't follow the format I am using in my tree ... converting Subject: lines is so trivial that I have never felt like bothering anyone about it ... it's basically single condition in a shellscript). Or the
It's slightly more than that if you're dealing with more than one area, and I also find this sort of stuff is a good flag for scrubbing the patch in greater detail - when patches stand out from a 1000ft visual overview there's a fair chance that there's other issues so if people regularly submit good patches that have only cosmetic issues I find it's worth guiding them away from that.
maintainer doesn't feel like the patch is worth it, and then the subject-line format really doesn't matter.
In this case if I don't apply it it's likely to end up going in via your tree and then I'll still have to deal with the merge conflicts which are more annoying.
On Nov 17 Mark Brown wrote:
On Wed, Nov 17, 2010 at 01:53:35AM +0100, Jiri Kosina wrote:
On Wed, 17 Nov 2010, Stefan Richter wrote:
Why should we codify our conventions in MAINTAINERS to accommodate the specific problem of virtually a _single_ patch author?
It seems to be the way we're heading in general - look at all the recent work on MAINTAINERS and get_maintainer.pl. There seems to be a genral push to make all this stuff automatable.
get_maintainer.pl, used with judgment and together with "gitk the/patched/source.c" is nice not only for people like Joe who regularly work tree-wide but also for ones like me who only rarely want to submit a bug report or patch for a subsystem with they are unfamiliar with.
But the thought of a database of "how to start a good patch title" is far-fetched. Really, as a patch author, just look how other people write patch titles and judge whether this is good for your work too or not.
Either the maintainer wants the patch. Then he is certainly able to apply it no matter the subject line (I personally am getting a lot of patches which don't follow the format I am using in my tree ... converting Subject: lines is so trivial that I have never felt like bothering anyone about it ... it's basically single condition in a shellscript). Or the
It's slightly more than that if you're dealing with more than one area, and I also find this sort of stuff is a good flag for scrubbing the patch in greater detail - when patches stand out from a 1000ft visual overview there's a fair chance that there's other issues so if people regularly submit good patches that have only cosmetic issues I find it's worth guiding them away from that.
On one hand Jiri is right that maintainers can adjust title prefixes ad hoc. (Downside: Weaker connection to mailinglist archives.) On the other hand, in the case of long-term prolific authors like Joe it is more optimal if there is a good patch title right from the outset.
So, if this boring thread does at least yield the conclusion that ${path}/${filename}: is a bad title prefix, at least something was won. :-)
Another thought: Whether a typical part of a mass conversion, e.g. to use a new helper macro without change of functionality, is named
[PATCH] [subsystem] driver: use foo_bar helper or [PATCH] use foo_bar helper in subsystem, driver
does not really matter, does it? This change is more about the helper than about the driver. It is really a different kind of changeset than a functional change that we want to be called
[PATCH] [subsystem] driver: fix crash at disconnection
or so. This is something that those who look for release notes of that driver or subsystem want to grep in the changelog.
Or in other words: If you as patch author wonder what would be a good title for your patch, then ask yourself: How should this change show up in kernel release notes that are constructed from the git shortlog? Sometimes the answer to this question includes among else a prefix with a canonical subsystem name (even case sensitive, with brackets or colon), whereas other times such formalities are utterly pointless.
[Sorry for the spent electrons. But OTOH, issues like (1.) optimum use of reviewer bandwidth, (2.) kernel changelog alias release notes /do/ matter.]
On Wed, 2010-11-17 at 22:07 +0100, Stefan Richter wrote:
So, if this boring thread does at least yield the conclusion that ${path}/${filename}: is a bad title prefix, at least something was won. :-)
I've changed my scripts to use this style:
Subject: [PATCH] $(basename $(dirname $file)): commit desc...
until a better tool is available.
On Mon 2010-11-15 09:34:04, Joe Perches wrote:
On Mon, 2010-11-15 at 17:30 +0000, Mark Brown wrote:
On Mon, Nov 15, 2010 at 09:09:17AM -0800, Joe Perches wrote:
Signed-off-by: Joe Perches joe@perches.com
Applied, thanks. Please try to use changelog formats consistent with the code you're modifying.
I think it's more important to use consistent changelogs for a patch series.
And I agree here. Having to learn code-style quirks for patches is bad, having to learn new changelog style for each subsystem is very bad. Pavel
participants (9)
-
Florian Mickler -
Jiri Kosina -
Joe Perches -
Liam Girdwood -
Mark Brown -
Pavel Machek -
Randy Dunlap -
Stefan Richter -
Takashi Iwai