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 <[email protected]>
---
v2 -> v3: Fixed spacing and newline in text
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.
--
2.34.1
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 <[email protected]>
> ---
> 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
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 <[email protected]>
>> ---
>> 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.
On Fri, Jan 19, 2024 at 06:42:57AM -0500, Jacob Lott wrote:
>
> 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.
I don't know anything about VSCode so I can't answer the question.
There aren't any tabs so really I can't imagine how VSCode could get the
col count wrong... (0_0)
regards,
dan carpenter