diff mbox series

[net-next,v3,09/13] doc/netlink: Regenerate netlink .rst files if ynl-gen-rst changes

Message ID 20231212221552.3622-10-donald.hunter@gmail.com (mailing list archive)
State Superseded
Delegated to: Netdev Maintainers
Headers show
Series tools/net/ynl: Add 'sub-message' support to ynl | expand

Checks

Context Check Description
netdev/series_format success Posting correctly formatted
netdev/tree_selection success Clearly marked for net-next
netdev/ynl success Generated files up to date; no warnings/errors; no diff in generated;
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 8 this patch: 8
netdev/cc_maintainers success CCed 2 of 2 maintainers
netdev/build_clang success Errors and warnings before: 8 this patch: 8
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 8 this patch: 8
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 15 lines checked
netdev/build_clang_rust success No Rust files in patch. Skipping build
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Donald Hunter Dec. 12, 2023, 10:15 p.m. UTC 
Add ynl-gen-rst.py to the dependencies for the netlink .rst files in the
doc Makefile so that the docs get regenerated if the ynl-gen-rst.py
script is modified. Use $(Q) to honour V=1 in these rules.

Signed-off-by: Donald Hunter <donald.hunter@gmail.com>
Reviewed-by: Jakub Kicinski <kuba@kernel.org>
---
 Documentation/Makefile | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

Comments

Breno Leitao Dec. 12, 2023, 11:34 p.m. UTC | #1 
> +$(YNL_INDEX): $(YNL_RST_FILES) $(YNL_TOOL)
> +	$(Q)$(YNL_TOOL) -o $@ -x

Isn't $(YNL_INDEX) depending to $(YNL_TOOL) indirectly since it depends
on $(YNL_RST_FILES) ?

I mean, do you really need the line above?

> +$(YNL_RST_DIR)/%.rst: $(YNL_YAML_DIR)/%.yaml $(YNL_TOOL)
> +	$(Q)$(YNL_TOOL) -i $< -o $@
Donald Hunter Dec. 13, 2023, 9:42 a.m. UTC | #2 
Breno Leitao <leitao@debian.org> writes:

>> +$(YNL_INDEX): $(YNL_RST_FILES) $(YNL_TOOL)
>> +	$(Q)$(YNL_TOOL) -o $@ -x
>
> Isn't $(YNL_INDEX) depending to $(YNL_TOOL) indirectly since it depends
> on $(YNL_RST_FILES) ?
>
> I mean, do you really need the line above?

Sure, the transitive dependency is sufficient. I tend to add an explicit
dependency for a script that gets run in a target.

Happy to remove that change and respin if you prefer?
Breno Leitao Dec. 13, 2023, 2:58 p.m. UTC | #3 
On Wed, Dec 13, 2023 at 09:42:52AM +0000, Donald Hunter wrote:
> Breno Leitao <leitao@debian.org> writes:
> 
> >> +$(YNL_INDEX): $(YNL_RST_FILES) $(YNL_TOOL)
> >> +	$(Q)$(YNL_TOOL) -o $@ -x
> >
> > Isn't $(YNL_INDEX) depending to $(YNL_TOOL) indirectly since it depends
> > on $(YNL_RST_FILES) ?
> >
> > I mean, do you really need the line above?
> 
> Sure, the transitive dependency is sufficient. I tend to add an explicit
> dependency for a script that gets run in a target.
> 
> Happy to remove that change and respin if you prefer?

I would say it is preferred to remove unnecessary code.

Feel free to add the following line in the new version:

	Reviewed-by: Breno Leitao <leitao@debian.org>

Thanks for addressing this.
Jakub Kicinski Dec. 13, 2023, 4:39 p.m. UTC | #4 
On Wed, 13 Dec 2023 09:42:52 +0000 Donald Hunter wrote:
> Sure, the transitive dependency is sufficient. I tend to add an explicit
> dependency for a script that gets run in a target.
> 
> Happy to remove that change and respin if you prefer?

I can drop patch 9 when applying if that's what you mean.
No need to repost the sub-message support.
Donald Hunter Dec. 13, 2023, 5:04 p.m. UTC | #5 
On Wed, 13 Dec 2023 at 16:39, Jakub Kicinski <kuba@kernel.org> wrote:
>
> On Wed, 13 Dec 2023 09:42:52 +0000 Donald Hunter wrote:
> > Sure, the transitive dependency is sufficient. I tend to add an explicit
> > dependency for a script that gets run in a target.
> >
> > Happy to remove that change and respin if you prefer?
>
> I can drop patch 9 when applying if that's what you mean.
> No need to repost the sub-message support.

No, it's one line of patch 9 that needs to be dropped.

> +$(YNL_INDEX): $(YNL_RST_FILES) $(YNL_TOOL)

The other three lines should remain.

I'll respin if you prefer.
Jakub Kicinski Dec. 13, 2023, 6:01 p.m. UTC | #6 
On Wed, 13 Dec 2023 17:04:11 +0000 Donald Hunter wrote:
> > I can drop patch 9 when applying if that's what you mean.
> > No need to repost the sub-message support.  
> 
> No, it's one line of patch 9 that needs to be dropped.
> 
> > +$(YNL_INDEX): $(YNL_RST_FILES) $(YNL_TOOL)  
> 
> The other three lines should remain.
> 
> I'll respin if you prefer.

Yeah, nah, I'm not editing the patch itself :)
diff mbox series

Patch

diff --git a/Documentation/Makefile b/Documentation/Makefile
index 5c156fbb6cdf..5f36a392ddfc 100644
--- a/Documentation/Makefile
+++ b/Documentation/Makefile
@@ -105,11 +105,11 @@  YNL_TOOL:=$(srctree)/tools/net/ynl/ynl-gen-rst.py
 YNL_RST_FILES_TMP := $(patsubst %.yaml,%.rst,$(wildcard $(YNL_YAML_DIR)/*.yaml))
 YNL_RST_FILES := $(patsubst $(YNL_YAML_DIR)%,$(YNL_RST_DIR)%, $(YNL_RST_FILES_TMP))
 
-$(YNL_INDEX): $(YNL_RST_FILES)
-	@$(YNL_TOOL) -o $@ -x
+$(YNL_INDEX): $(YNL_RST_FILES) $(YNL_TOOL)
+	$(Q)$(YNL_TOOL) -o $@ -x
 
-$(YNL_RST_DIR)/%.rst: $(YNL_YAML_DIR)/%.yaml
-	@$(YNL_TOOL) -i $< -o $@
+$(YNL_RST_DIR)/%.rst: $(YNL_YAML_DIR)/%.yaml $(YNL_TOOL)
+	$(Q)$(YNL_TOOL) -i $< -o $@
 
 htmldocs: $(YNL_INDEX)
 	@$(srctree)/scripts/sphinx-pre-install --version-check