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

Don't use a single type in MockScriptHandler constructor #541

Merged
merged 2 commits into from
Jul 11, 2022

Conversation

nre-ableton
Copy link
Contributor

Instead of trying to squeeze three values in one Object type, this change introduces more constructor overrides for the MockScriptHandler class.

Note that this will be an API-breaking change that we'll need to document when released.

Fixes #536.

  • Make sure you are opening from a topic/feature/bugfix branch (right side) and not your main branch!
  • Ensure that the pull request title represents the desired changelog entry
  • Please describe what you did
  • Link to relevant issues in GitHub or Jira
  • Link to relevant pull requests, esp. upstream and downstream changes
  • Ensure you have provided tests - that demonstrates feature works or fixes the issue

The previous implementation of the MockScriptHandler constructor was a
bit brittle and confusing. The first parameter was an Object which could
either be null (for no pattern matching), a String (for full pattern
matching), or a Pattern (for a regexp pattern match).

This change now provides three different constructors for the three
different use-cases detailed above, and also creates separate fields for
the various matches. This means we no longer need to rely on instanceof
to see what type of filter should be applied.
@nre-ableton
Copy link
Contributor Author

@lemeurherve I've invited you as a reviewer, in case you are interested in seeing this change.

@nre-ableton nre-ableton merged commit 38aaffe into master Jul 11, 2022
@nre-ableton nre-ableton deleted the nre/master/536-regexp-as-first-argument branch July 11, 2022 16:34
@lemeurherve
Copy link
Member

Sorry I missed that one, thanks a lot @nre-ableton

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.

Add override to helper.addShMock to accept a regexp as first argument
2 participants