Skip to content

Conversation

@boegel
Copy link
Member

@boegel boegel commented Jun 14, 2021

fix incorrect statement I introduced in #714 🙊

Copy link
Contributor

@akesandgren akesandgren left a comment

Choose a reason for hiding this comment

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

LGTM

* the filepath of the module that will be written
* the module text as a string
The return value of this hook (if any) will then be appended to the contents of the module file.
The return value of this hook (if any) is used as contents for the module file that will be written.
Copy link
Contributor

Choose a reason for hiding this comment

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

TBH: I find "as contents for the file that will be written" a bit to complicated compared to "will be written to the file"
Maybe we could still clarify this a bit, especially with respect to "no return value/no hook":

Suggested change
The return value of this hook (if any) is used as contents for the module file that will be written.
The return value of this hook, when set, will replace the original text that is then written to the module file.

Copy link
Member Author

Choose a reason for hiding this comment

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

That's better indeed, opening another PR :)

Copy link
Member Author

Choose a reason for hiding this comment

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

see #716

@akesandgren
Copy link
Contributor

Going in, thanks @boegel!

@akesandgren akesandgren merged commit f7ff8d0 into easybuilders:develop Jun 14, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants