| Message ID | 20240612193831.25913-2-davthompson@nvidia.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Style fixes for EDAC/bluefield driver | expand |
On Wed, Jun 12, 2024 at 03:38:30PM -0400, David Thompson wrote: > This patch removes an empty line in bluefield_edac_mc_probe(). Avoid having "This patch" or "This commit" in the commit message. It is tautologically useless. Also, do $ git grep 'This patch' Documentation/process for more details. Also, feel free to peruse that whole directory. > Signed-off-by: David Thompson <davthompson@nvidia.com> > Reviewed-by: Shravan Kumar Ramani <shravankr@nvidia.com> > --- > drivers/edac/bluefield_edac.c | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/drivers/edac/bluefield_edac.c b/drivers/edac/bluefield_edac.c > index 5b3164560648..1f6f39a7dbf3 100644 > --- a/drivers/edac/bluefield_edac.c > +++ b/drivers/edac/bluefield_edac.c > @@ -320,7 +320,6 @@ static int bluefield_edac_mc_probe(struct platform_device *pdev) > edac_mc_free(mci); > > return ret; > - > } > > static void bluefield_edac_mc_remove(struct platform_device *pdev) > -- So just the effort to create a whole patch just for that is an overkill. Please do not do that. If you notice very minor style issues like that, you can do them when touching this code as part of a change with more substance. Or you can simply ignore such minor issues. Whitespace cleanup like that gets in the way of real work and pretty much all maintainers are overworked already. So I'd appreciate it if you concentrate on real fixes and improvements. Thx.
> -----Original Message----- > From: Borislav Petkov <bp@alien8.de> > Sent: Wednesday, June 12, 2024 4:01 PM > To: David Thompson <davthompson@nvidia.com> > Cc: tony.luck@intel.com; james.morse@arm.com; mchehab@kernel.org; > rric@kernel.org; Shravan Ramani <shravankr@nvidia.com>; linux- > edac@vger.kernel.org; linux-kernel@vger.kernel.org > Subject: Re: [PATCH v1 1/2] EDAC/bluefield: fix white space in > bluefield_edac_mc_probe > > On Wed, Jun 12, 2024 at 03:38:30PM -0400, David Thompson wrote: > > This patch removes an empty line in bluefield_edac_mc_probe(). > > Avoid having "This patch" or "This commit" in the commit message. It is > tautologically useless. > > Also, do > > $ git grep 'This patch' Documentation/process > > for more details. > I did review this section of the kernel documentation and then went on to create a v2 with the updated commit message. I apologize but I didn't notice your comment below about your preference to not have this type of patch at all. I have been told in the past to have separate patches for style cleanups, and not to bundle them with features. But I can do as you recommend for next version. I will squash the two style fixes into a patch that contains some new functionality for the BlueField EDAC driver. > Also, feel free to peruse that whole directory. > > > Signed-off-by: David Thompson <davthompson@nvidia.com> > > Reviewed-by: Shravan Kumar Ramani <shravankr@nvidia.com> > > --- > > drivers/edac/bluefield_edac.c | 1 - > > 1 file changed, 1 deletion(-) > > > > diff --git a/drivers/edac/bluefield_edac.c > > b/drivers/edac/bluefield_edac.c index 5b3164560648..1f6f39a7dbf3 > > 100644 > > --- a/drivers/edac/bluefield_edac.c > > +++ b/drivers/edac/bluefield_edac.c > > @@ -320,7 +320,6 @@ static int bluefield_edac_mc_probe(struct > platform_device *pdev) > > edac_mc_free(mci); > > > > return ret; > > - > > } > > > > static void bluefield_edac_mc_remove(struct platform_device *pdev) > > -- > > So just the effort to create a whole patch just for that is an overkill. > > Please do not do that. If you notice very minor style issues like that, you can do > them when touching this code as part of a change with more substance. Or you > can simply ignore such minor issues. > > Whitespace cleanup like that gets in the way of real work and pretty much all > maintainers are overworked already. > > So I'd appreciate it if you concentrate on real fixes and improvements. > > Thx. > > -- > Regards/Gruss, > Boris. > > https://people.kernel.org/tglx/notes-about-netiquette
On Thu, Jun 13, 2024 at 03:06:38PM +0000, David Thompson wrote: > I did review this section of the kernel documentation and then > went on to create a v2 with the updated commit message. > I apologize but I didn't notice your comment below about > your preference to not have this type of patch at all. I have been > told in the past to have separate patches for style cleanups, and Yeah, it all depends on the maintainer. I, personally, do not encourage style cleanups only because they interfere with git archeology and teach new submitters not to chase real bugs but do silly patches only. > not to bundle them with features. But I can do as you recommend for next > version. I will squash the two style fixes into a patch that contains some > new functionality for the BlueField EDAC driver. Thanks. Simply use checkpatch on your patches only but not on the .c files directly.
diff --git a/drivers/edac/bluefield_edac.c b/drivers/edac/bluefield_edac.c index 5b3164560648..1f6f39a7dbf3 100644 --- a/drivers/edac/bluefield_edac.c +++ b/drivers/edac/bluefield_edac.c @@ -320,7 +320,6 @@ static int bluefield_edac_mc_probe(struct platform_device *pdev) edac_mc_free(mci); return ret; - } static void bluefield_edac_mc_remove(struct platform_device *pdev)