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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Alternate approach to hooks documentation #145

Merged
merged 4 commits into from Oct 15, 2017

Conversation

Projects
None yet
2 participants
@wasnotrice
Collaborator

wasnotrice commented Oct 15, 2017

Hi @PragTob, I promised an example of the hooks documentation suggestions I made in #143, so here it is. I tried to name a few more concepts and structure things by Suite/Scenario/Benchmarking rather than by the order in which all the functions appear. I also tried to keep the explanation for each kind of hook brief, with cross-references to sections on arguments/ordering where those confusing parts are explained in more detail. I did not change any of the code examples.

Feel free to take what you like and discard what you don't 馃榾

PragTob and others added some commits Oct 12, 2017

Documentation for the new hooks system
* already includes not yet merged `after_scenario` and its handling
* this is basically a blog post _sigh_ - you only realize how
  complex the system you built in your head is when you document
  it :)
* honestly I'm on my way to the beach now so didn't get to reading
  it through another time myself, sorry :D
* Feedback very welcome, on strucutre/order/content whatever - I
  want this documentation to be as good as it can get because
  that's what a proper library should be doing.
@PragTob

Yum yum Ghosti likes this approach!

img_20170926_164012

Gotta say I like this better than the other one, thanks a ton 馃帀 Besides the structure change that makes sense it also has multiple small detail improvements to layout/structure and it also makes quite a few passages more succinct!

Some small questions/requests but I could also take care of them myself.

Thanks @wasnotrice 馃憦

README.md Outdated
@@ -219,19 +219,20 @@ Benchee.run(%{
### Hooks (Setup, Teardown etc.)
If you want to do something before or after something happens in benchee, we got you coverd! Before you dig into this, **most of the times you shouldn't actually need this**. When you are just benchmarking plain old immutable functions no hooks whatsoever should be needed. Hooks are mostly needed when you need side effects to happen, randomize something or something similar.
Most of the time, it's best to keep your benchmarks as simple as possible: plain old immutable functions work best. But sometimes you need other things to happen. When you want to add before or after hooks to your benchmarks, we've got you covered! Before you dig into this section, **you ususally don't need hooks**.

This comment has been minimized.

@PragTob

PragTob Oct 15, 2017

Owner

馃憤 Sounds good

README.md Outdated
* each individual invocation of a benchmarking function - `before_each`/`after_each`
* [Suite hooks](#suite-hooks)
* [Scenario hooks](#scenario-hooks)
* [Benchmarking function hooks](#benchmarking-function-hooks)

This comment has been minimized.

@PragTob

PragTob Oct 15, 2017

Owner

馃憤 linking the sections

README.md Outdated
@@ -241,77 +242,50 @@ Benchee.run %{"Awesome stuff" => fn -> magic end }
your_teardown()
```
When might this be useful?
_When might this be useful?_

This comment has been minimized.

@PragTob

PragTob Oct 15, 2017

Owner

Like the italicization! 馃憤

README.md Outdated
* you want to alter the given input, in that case alter the given input
* you want to keep the given input but add some other data, in that case just return a tuple like `{original_input, new_fancy_data}`
* you just want to invoke a side effect: in that case return the given input unchanged
* you want to alter the given input: in that case alter the given input

This comment has been minimized.

@PragTob

PragTob Oct 15, 2017

Owner

newline inside the list seems odd?

README.md Outdated
### after_scenario
#### Hook arguments and return values
Before hooks form a chain, where the return value of the previous hook becomes the argument for the next one. The first defined `before` hook receives the scenario input as an argument, and returns a value that becomes the argument of the next in the chain. The benchmarking function receives the value of the last `before` hook as its argument (or the scenario input if there are no `before` hooks).

This comment has been minimized.

@PragTob

PragTob Oct 15, 2017

Owner

馃憤

README.md Outdated
Is executed after a scenario completed and is passed whatever the return value of the last `before_scenario` was, if there was none it is passed the specified `input`. Otherwise it works much like [`after_each`](#after_each).
After hooks do not form a chain, and their return values are discarded. An `after_each` hook receives the return value of the benchmarking function as its argument. An `after_scenario` function receives the return value of its `before_scenario` counterpart (or the scenario's input if there is no `before_scenario` hook).

This comment has been minimized.

@PragTob

PragTob Oct 15, 2017

Owner

Slightly incorrect here (and also in the before_scenario) part - the after_scenario receives the output of the last before_scenario as an input. Aka if there is only a global one both global and local receives its output, if there is a local one both receive that.

We don't wanna enforce that there always needs to be a counterpart, so they just get the return value of the before_scenario the furthest down the chain

README.md Outdated
#### When does a hook happen? (Complete Example)
Yes the whole hooks system is quite a lot to take in. Here is an overview when what hook is executed with what input depending on its previous hooks. In general, if some hook isn't specified the value of the _previous_ hook is passed on.
Yes the whole hooks system is quite a lot to take in. Here is an overview showing the order of hook execution, along with the argument each hook receives (see [hook arguments and return values](#hook-arguments-and-return-values)).

This comment has been minimized.

@PragTob

PragTob Oct 15, 2017

Owner

For the whole is "global or local invoked first" we also might mention the guiding principle, which is: "hooks defined locally execute closer to the benchmarking function" (which is why it swtiches between before_ and after_)

@wasnotrice

This comment has been minimized.

Collaborator

wasnotrice commented Oct 15, 2017

@PragTob glad you like it! I agree with all of your comments as well. Feel free to make the changes if you get to it before I do :)

@PragTob

This comment has been minimized.

Owner

PragTob commented Oct 15, 2017

@wasnotrice assuming you don't have time atm I'll go ahead and make them, then merge so we can get it moving fairly quickly. A release is somewhere along the line but still some loose ends to tie up :)

@PragTob

This comment has been minimized.

Owner

PragTob commented Oct 15, 2017

README only --> no CI needed.

We can still do new fixups from master :)

@PragTob PragTob merged commit c697596 into master Oct 15, 2017

@PragTob PragTob deleted the hooks-documentation-alt branch Oct 15, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment