Update documentation around non-capturing monadic bind for Action #849
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
cdepillabout
commented
May 4, 2024
| -- will have its dependencies run in parallel. For example @'need' [\"a\"] *> 'need [\"b\"]@ is equivalent | ||
| -- to @'need' [\"a\", \"b\"]@. | ||
| -- will have its dependencies run in parallel. For example, | ||
| -- @'Development.Shake.Internal.Rules.File.need' [\"a\"] '*>' 'Development.Shake.Internal.Rules.File.need' [\"b\"]@ |
There was a problem hiding this comment.
I needed to fully qualify the path to need here, since it appears that Haddock can't figure it out:
https://hackage.haskell.org/package/shake-0.19.8/docs/Development-Shake.html#t:Action
See how the needs on the last line here aren't actually links, since Haddock doesn't seem to know where it should link to? Writing these as fully-qualified references should fix this.
cdepillabout
commented
May 4, 2024
Comment on lines
-374
to
+380
| -- Usually @need [foo,bar]@ is preferable to @need [foo] >> need [bar]@ as the former allows greater | ||
| -- parallelism, while the latter requires @foo@ to finish building before starting to build @bar@. | ||
| -- Note that the following expressions are all equivalent. @foo@ and @bar@ are built in parallel: | ||
| -- | ||
| -- - @need [foo,bar]@ | ||
| -- - @need [foo] '*>' need [bar]@ | ||
| -- - @need [foo] '>>' need [bar]@ | ||
| -- | ||
| -- In this situation, @need [foo, bar]@ is preferable, since the parallelism is the most obvious. |
There was a problem hiding this comment.
I had also wanted to add a note here saying something like:
If you do require that two separate
needs are built sequentially, you can put another (non-need)Actionin between, like the following:do need [foo] liftIO (putStr "") need [bar]
However:
- I wasn't sure what would be the best way to write this "no-op" Action (
liftIO putStrseems kind of hacky) - This isn't something that just affects
need, it also affects all the other actions that useneedinternally, likereadFileLines.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR fixes up some of the documentation around the interaction of the non-capturing monadic bind (
>>) forAction, and theneedAction.From https://neilmitchell.blogspot.com/2019/05/shake-with-applicative-parallelism.html, it appears that since Shake 0.18,
need [foo] >> need [bar]is intepreted the same way as bothneed [foo, bar], andneed [foo] *> need [bar]. There were a few places left in the Haddocks that weren't yet updated about this, so I've fixed this in this PR.