| Message ID | 20240409071902.514706-1-amorenoz@redhat.com |
|---|---|
| State | Accepted |
| Delegated to: | Simon Horman |
| Headers | show |
| Series | [ovs-dev] checkpatch: Allow rST manpages to be added. | expand |
| Context | Check | Description |
|---|---|---|
| ovsrobot/apply-robot | success | apply and check: success |
| ovsrobot/github-robot-_Build_and_Test | success | github build: passed |
On Tue, Apr 09, 2024 at 09:19:02AM +0200, Adrian Moreno wrote: > The current __check_doc_is_listed() verifies that the new .rst file is > listed in Documentation/automake.mk with the full path (i.e: > "{directory}/{filename}"). > > While this holds true for generic documentation files, which are added > to DOC_SOURCE with the full path, it's not true for rST manpages which > are added only by filename to RST_MANPAGES target (see > Documentation/automake.mk). > > This makes the current implementation of the check to incorrectly raise > a warning as follows even though the patch does add the file to > RST_MANPAGES: > > """ > WARNING: New doc ovs-flowviz.8.rst not listed in > Documentation/automake.mk > """ > > Fix it by making the {dir}/ part of the docre regexp optional. > > Signed-off-by: Adrian Moreno <amorenoz@redhat.com> Acked-by: Simon Horman <horms@ovn.org>
On 9 Apr 2024, at 9:19, Adrian Moreno wrote: > The current __check_doc_is_listed() verifies that the new .rst file is > listed in Documentation/automake.mk with the full path (i.e: > "{directory}/{filename}"). > > While this holds true for generic documentation files, which are added > to DOC_SOURCE with the full path, it's not true for rST manpages which > are added only by filename to RST_MANPAGES target (see > Documentation/automake.mk). > > This makes the current implementation of the check to incorrectly raise > a warning as follows even though the patch does add the file to > RST_MANPAGES: > > """ > WARNING: New doc ovs-flowviz.8.rst not listed in > Documentation/automake.mk > """ > > Fix it by making the {dir}/ part of the docre regexp optional. > > Signed-off-by: Adrian Moreno <amorenoz@redhat.com> Thanks for fixing this! The change looks good to me. Acked-by: Eelco Chaudron <echaudro@redhat.com>
On Wed, Apr 10, 2024 at 03:45:55PM +0200, Eelco Chaudron wrote: > > > On 9 Apr 2024, at 9:19, Adrian Moreno wrote: > > > The current __check_doc_is_listed() verifies that the new .rst file is > > listed in Documentation/automake.mk with the full path (i.e: > > "{directory}/{filename}"). > > > > While this holds true for generic documentation files, which are added > > to DOC_SOURCE with the full path, it's not true for rST manpages which > > are added only by filename to RST_MANPAGES target (see > > Documentation/automake.mk). > > > > This makes the current implementation of the check to incorrectly raise > > a warning as follows even though the patch does add the file to > > RST_MANPAGES: > > > > """ > > WARNING: New doc ovs-flowviz.8.rst not listed in > > Documentation/automake.mk > > """ > > > > Fix it by making the {dir}/ part of the docre regexp optional. > > > > Signed-off-by: Adrian Moreno <amorenoz@redhat.com> > > Thanks for fixing this! The change looks good to me. > > Acked-by: Eelco Chaudron <echaudro@redhat.com> Thanks Adrian and Eelco, Applied to main. - checkpatch: Allow rST manpages to be added. https://github.com/openvswitch/ovs/commit/e1e0c6a3ed51
diff --git a/utilities/checkpatch.py b/utilities/checkpatch.py index e0cab6b9f..48fb35ff5 100755 --- a/utilities/checkpatch.py +++ b/utilities/checkpatch.py @@ -479,7 +479,7 @@ def __check_doc_is_listed(text, doctype, docdir, docfile): docre = re.compile(r'\n\+.*{}'.format(docfile.replace('.rst', ''))) elif doctype == 'automake': beginre = re.compile(r'\+\+\+.*Documentation/automake.mk') - docre = re.compile(r'\n\+\t{}/{}'.format(docdir, docfile)) + docre = re.compile(r'\n\+\t(?:{}/)?{}'.format(docdir, docfile)) else: raise NotImplementedError("Invalid doctype: {}".format(doctype))
The current __check_doc_is_listed() verifies that the new .rst file is listed in Documentation/automake.mk with the full path (i.e: "{directory}/{filename}"). While this holds true for generic documentation files, which are added to DOC_SOURCE with the full path, it's not true for rST manpages which are added only by filename to RST_MANPAGES target (see Documentation/automake.mk). This makes the current implementation of the check to incorrectly raise a warning as follows even though the patch does add the file to RST_MANPAGES: """ WARNING: New doc ovs-flowviz.8.rst not listed in Documentation/automake.mk """ Fix it by making the {dir}/ part of the docre regexp optional. Signed-off-by: Adrian Moreno <amorenoz@redhat.com> --- utilities/checkpatch.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)