Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

make: fix validate-docs #1376

Merged
merged 1 commit into from
Mar 17, 2021
Merged

make: fix validate-docs #1376

merged 1 commit into from
Mar 17, 2021

Conversation

dmke
Copy link
Member

@dmke dmke commented Mar 17, 2021

In #1374 the PDNS documentation was amended, but the author forgot to re-generate the docs.

We do have a validation check in the main workflow, but it didn't catch the change because the underlying Makefile target was broken.

This commit replaces the Makefile conditional with an equivalent shell conditional.

Explanation

The conditional checking for a dirty working directory ran before generating the documentation. In effect, make validate-doc only
did the right thing on the second invokation.

Consider this simplified Makefile example:

generate:
	aaaa
	
validate: generate
ifneq ($(shell bbbb),)
	cccc
else
	dddd
endif

The order of evaluation for make validate is:

  1. detect generate target, no dependencies,
  2. set body for generate target to aaaa,
  3. detect validate target, with dependency generate,
  4. execute bbbb, compare with empty string,
    • if not empty: set body for validate target to cccc
    • else: set body for validate target to dddd
  5. invoke generate → execute aaaa (generate is a dependency of validate),
  6. invoke validate → execute cccc or dddd.

This also means that step 4 is also (unnecessarily) executed when the validate target is never invoked.

This commit replaces a make conditional with an equivalent
shell conditional.

The conditional checking for a dirty working directory ran before
generating the documentation. In effect, `make validate-doc` only
did the right thing on the second invokation.
@dmke dmke requested a review from ldez March 17, 2021 00:46
@ldez ldez added this to the v4.4 milestone Mar 17, 2021
Copy link
Member

@ldez ldez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@ldez ldez merged commit ee0b4bd into go-acme:master Mar 17, 2021
@dmke dmke deleted the fix/validate-docs branch January 21, 2023 20:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

2 participants