Re: [PATCH v3] staging: rts5208: Add more details to Kconfig help

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 1/19/24 1:34 AM, Dan Carpenter wrote:
On Thu, Jan 18, 2024 at 02:17:04PM -0500, Jacob Lott wrote:
The current help text is short and triggers a
warning from checkpatch.pl. This patch adds more
details to the help text which should provide better
information for whether or not to enable the driver.

Signed-off-by: Jacob Lott <jklott.git@xxxxxxxxx>
---
v2 -> v3: Fixed spacing and newline in text
The commit message is still not line wrapped correctly at 72-75
characters.  It should have been:

The current help text is short and triggers a warning from checkpatch.pl.
This patch adds more details to the help text which should provide better
information for whether or not to enable the driver.

Or:

The current help text is short and triggers a warning from
checkpatch.pl. This patch adds more details to the help text which
should provide better information for whether or not to enable the
driver.


v1 -> v2: Corrected line spacing based off feedback

  drivers/staging/rts5208/Kconfig | 10 +++++++---
  1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/rts5208/Kconfig b/drivers/staging/rts5208/Kconfig
index b864023d3ccb..c4664a26ba3b 100644
--- a/drivers/staging/rts5208/Kconfig
+++ b/drivers/staging/rts5208/Kconfig
@@ -3,7 +3,11 @@ config RTS5208
  	tristate "Realtek PCI-E Card Reader RTS5208/5288 support"
  	depends on PCI && SCSI
  	help
-	  Say Y here to include driver code to support the Realtek
-	  PCI-E card reader rts5208/rts5288.
+	  Choose Y here to enable support for the Realtek PCI-E card reader
+	  RTS5208/5288. This driver facilitates communication between the Linux
+	  kernel and the Realtek PCI-E card reader.
- If this driver is compiled as a module, it will be named rts5208.
+	  If you opt to compile this driver as a module, it will be named rts5208.
+	  Selecting N will exclude this driver from the kernel build. Choose option
+	  Y if your system includes the Realtek PCI-E card reader rts5208/rts5288.
+	  When in doubt, it is generally safe to select N.

In the original code there were three lines of help text.

1)         Say Y here to include driver code to support the Realtek
2)         PCI-E card reader rts5208/rts5288.
3)         If this driver is compiled as a module, it will be named rts5208.

The minimum that checkpatch wants is 4 lines.

However, I feel like nothing you're adding here is useful information.
Some of it is just confusing like, "This driver facilitates
communication between the Linux kernel and the Realtek PCI-E card
reader."  That line feels like information, but when you think about it,
it doesn't actually mean anything.  The second paragraph is just
repeated information so it's kind of a waste of time.

Don't just obey checkpatch for the sake of obeying, if we can't think
of anything else useful to say then three lines is enough.

regards,
dan carpenter

Thank you for your feedback. Based off what Dan has said I would agree that it is best to leave the information as is.


For the future when talking about line wrapping I've been going based off VSCode's row/col count, is this not correct? For reference when writing the commit I would use a new line before hitting 72 "col" on each row. I ask for the future so I can make better commits.





[Index of Archives]     [Linux Driver Development]     [Linux Driver Backports]     [DMA Engine]     [Linux GPIO]     [Linux SPI]     [Video for Linux]     [Linux USB Devel]     [Linux Coverity]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux