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

Change default tape mode #11

Merged

Conversation

caioceccon
Copy link
Contributor

Add support to change the default tape mode be calling Walkman.set_global_default(true).

I have initially thought about doing something like:
@spec set_default_tape_mode(mode :: :global | :local) :: :ok

But because it would be different from the options used by use_tape macro which allow to pass the options global: true, or false I have implemented as in:
@spec set_global_default(default_value :: true | false) :: :ok

Please let me know if is there preference for one over the other. I believe that tape mode or type with possible values as :global and :local would make more sense but then would be better to make the macro use_tape consistent and use the same possible name and value for accepted options.

The macro was capturing the exceptions and causing a failing test to
pass when the exception was happening before the asserts statements.
@derekkraan
Copy link
Owner

Thanks for PR! I think it might make sense to configure this using application configuration. So that in config/test.exs you'd put something like config :walkman, global: true

What do you think?

@caioceccon caioceccon force-pushed the feature/change-default-tape-mode branch 2 times, most recently from 1a8283f to 58781d9 Compare October 17, 2019 08:20
@caioceccon caioceccon force-pushed the feature/change-default-tape-mode branch from 58781d9 to d68efdc Compare October 17, 2019 08:21
@derekkraan derekkraan merged commit 836cc4f into derekkraan:master Oct 17, 2019
@derekkraan
Copy link
Owner

Thanks @caioceccon , looks great!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants