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

Allow hiding 'on_load' attribute. #131

Merged
merged 2 commits into from
Nov 25, 2014
Merged

Conversation

rlipscombe
Copy link
Contributor

See #130.

hide_on_load() ->
% We _don't_ want on_load to be called. Listen out for it.
register(on_load_listener, self()),
meck:new(meck_on_load_module, [passthrough, hide_on_load]),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

If you remove hide_on_load, then make test fails with the following:

meck_on_load_tests: on_load_test_...*failed*
in function meck_on_load_tests:hide_on_load/0 (test/meck_on_load_tests.erl, line 21)
**error:unexpected_call_to_on_load

@eproxus
Copy link
Owner

eproxus commented Nov 25, 2014

Hi, thank you so much for the PR. I'm not really a fan of the name hide_on_load. I have two other options, which one do you like the best?

  • disable_on_load (defaults to true)
  • backup_on_load (defaults to false)

I particularly like the second one because it also signals that it has something to do with the backup modules.

@rlipscombe
Copy link
Contributor Author

I prefer the first one. I don't think that people writing the test particularly care about backup or not. They just don't want on_load to be called.

Except that {disable_on_load, false} is a double-negative. Maybe enable_on_load, default to false?

@eproxus
Copy link
Owner

eproxus commented Nov 25, 2014

Agree, double-negatives are bad. Would you mind changing to enable_on_load?

eproxus added a commit that referenced this pull request Nov 25, 2014
Allow hiding 'on_load' attribute.
@eproxus eproxus merged commit 1ab5e23 into eproxus:master Nov 25, 2014
@eproxus
Copy link
Owner

eproxus commented Nov 25, 2014

Again, thanks a lot!

@rlipscombe
Copy link
Contributor Author

No problem.

@rlipscombe rlipscombe deleted the rl-hide-on-load branch November 25, 2014 15:40
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.

None yet

2 participants