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

Remove warnings #124

Merged
merged 3 commits into from
Feb 14, 2021
Merged

Remove warnings #124

merged 3 commits into from
Feb 14, 2021

Conversation

mcmire
Copy link
Collaborator

@mcmire mcmire commented Feb 14, 2021

When running tests locally, multiple warnings were being emitted. A lot
of these warnings have to do with the fact that we are overriding core
methods in RSpec, and whenever you override a method, Ruby feels the
need to inform you. To get around this, we need to use the same trick
that we already use for AugmentedMatcher: we create a module on the fly
and prepend it to the class or module in question. Sometimes we might
even need to prepend to the singleton class. Also, this commit adds
WarningsLogger so that we can track when this happens again.

@mcmire mcmire force-pushed the remove-warnings branch 6 times, most recently from a38aaff to b697aa4 Compare February 14, 2021 05:50
This ends up being really annoying each time. `appraisal install` is
already in the `setup` script so that should be the preferred method of
installing all dependencies.
When running tests locally, multiple warnings were being emitted. A lot
of these warnings have to do with the fact that we are overriding core
methods in RSpec, and whenever you override a method, Ruby feels the
need to inform you. To get around this, we need to use the same trick
that we already use for AugmentedMatcher: we create a module on the fly
and `prepend` it to the class or module in question. Sometimes we might
even need to `prepend` to the singleton class. Also, this commit adds
WarningsLogger so that we can track when this happens again.
@mcmire mcmire merged commit b2e82a1 into master Feb 14, 2021
@mcmire mcmire deleted the remove-warnings branch February 14, 2021 07:31
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.

1 participant