diff mbox series

[v3,3/3] doc/guides: updated script usage for checking patches

Message ID 20200602135319.21457-4-ciara.power@intel.com (mailing list archive)
State Superseded, archived
Delegated to: Thomas Monjalon
Headers
Series standardize devtools check scripts |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/travis-robot success Travis build: passed
ci/Intel-compilation success Compilation OK

Commit Message

Power, Ciara June 2, 2020, 1:53 p.m. UTC 
  The contributor's guide includes the usage of both the checkpatches and
check-git-log scripts, which needed to be updated to reflect the now
standardised format.

Signed-off-by: Ciara Power <ciara.power@intel.com>
---
 doc/guides/contributing/patches.rst | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

Comments

Ferruh Yigit June 3, 2020, 3:50 p.m. UTC | #1 
On 6/2/2020 2:53 PM, Ciara Power wrote:
> The contributor's guide includes the usage of both the checkpatches and
> check-git-log scripts, which needed to be updated to reflect the now
> standardised format.
> 
> Signed-off-by: Ciara Power <ciara.power@intel.com>

Acked-by: Ferruh Yigit <ferruh.yigit@intel.com>
Thomas Monjalon June 17, 2020, 9:46 a.m. UTC | #2 
02/06/2020 15:53, Ciara Power:
> The contributor's guide includes the usage of both the checkpatches and
> check-git-log scripts, which needed to be updated to reflect the now
> standardised format.

I think the doc update should be merged in the first patch.
In general, doc and code updates should come together.

[...]
>  The script usage is::
>  
> -   checkpatches.sh [-h] [-q] [-v] [patch1 [patch2] ...]]"
> +   checkpatches.sh [-h] [-q] [-v] [-nX|-r range|patch1 [patch2] ...]"
>  
>  Where:
>  
>  * ``-h``: help, usage.
>  * ``-q``: quiet. Don't output anything for files without issues.
>  * ``-v``: verbose.
> +* ``-nX``: the number of commits to check.
> +* ``-r range``: the range to check, range must be a ``git log`` option.
>  * ``patchX``: path to one or more patches.

This is repeating what we get with the -h option.
I would suggest dropping the options explanation from this guide.

>  
>  Then the git logs should be checked using the ``check-git-log.sh`` script.
>  
>  The script usage is::
>  
> -   check-git-log.sh [range]
> +   check-git-log.sh [-h] [-nX|-r range]
>  
> -Where the range is a ``git log`` option.
> +Where:
> +
> +* ``-h``: help, usage.
> +* ``-nX``: the number of commits to check.
> +* ``-r range``: the range to check, range must be a ``git log`` option.

Same here, it is repeating what is documented with using -h.

If we really want to give an indication, we can say once for both that
the -n option is a number of commits from HEAD,
and -r option is a "git log" range.
A single sentence is probably enough.

In general, if we want guides to be read, they must be short.
John, as the doc owner, do you agree?
diff mbox series

Patch

diff --git a/doc/guides/contributing/patches.rst b/doc/guides/contributing/patches.rst
index 59442824a..f11e4d7c7 100644
--- a/doc/guides/contributing/patches.rst
+++ b/doc/guides/contributing/patches.rst
@@ -429,22 +429,28 @@  Once the environment variable the script can be run as follows::
 
 The script usage is::
 
-   checkpatches.sh [-h] [-q] [-v] [patch1 [patch2] ...]]"
+   checkpatches.sh [-h] [-q] [-v] [-nX|-r range|patch1 [patch2] ...]"
 
 Where:
 
 * ``-h``: help, usage.
 * ``-q``: quiet. Don't output anything for files without issues.
 * ``-v``: verbose.
+* ``-nX``: the number of commits to check.
+* ``-r range``: the range to check, range must be a ``git log`` option.
 * ``patchX``: path to one or more patches.
 
 Then the git logs should be checked using the ``check-git-log.sh`` script.
 
 The script usage is::
 
-   check-git-log.sh [range]
+   check-git-log.sh [-h] [-nX|-r range]
 
-Where the range is a ``git log`` option.
+Where:
+
+* ``-h``: help, usage.
+* ``-nX``: the number of commits to check.
+* ``-r range``: the range to check, range must be a ``git log`` option.
 
 
 .. _contrib_check_compilation: