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

Make touch honour if: and unless: options #1349

Closed
wants to merge 2 commits into from
Closed

Make touch honour if: and unless: options #1349

wants to merge 2 commits into from

Conversation

nrw505
Copy link
Contributor

@nrw505 nrw505 commented Oct 19, 2021

Thank you for your contribution!

Check the following boxes:

  • Wrote good commit messages.
  • Feature branch is up-to-date with master (if not - rebase it).
  • Squashed related commits together.
  • Added tests.
  • Added an entry to the Changelog if the new
    code introduces user-observable changes.
  • The PR relates to only one subject with a clear title
    and description in grammatically correct, complete sentences.

The on_touch callback was not honouring the if: or unless: options
that were set in the model config.
These tests were actually testing the opposite of what they said they
were testing.
@tlynam
Copy link
Contributor

tlynam commented Oct 29, 2021

I don't know this area well but this looks good to me, what do you think @jaredbeck? I think we'll want to squash the commits.

jaredbeck added a commit that referenced this pull request Nov 28, 2021
jaredbeck added a commit that referenced this pull request Nov 28, 2021
* Honour if: and unless: options in on_touch

The on_touch callback was not honouring the if: or unless: options
that were set in the model config.

* Fix some test descriptions

These tests were actually testing the opposite of what they said they
were testing.

* Doc: changelog entry for #1349

[ci skip]

Co-authored-by: Nigel Williams <nigel.williams@marketplacer.com>
@jaredbeck
Copy link
Member

Thanks Nigel, nice job on the tests.

Merged as #1354

@jaredbeck jaredbeck closed this Nov 28, 2021
@nrw505 nrw505 deleted the touch-does-not-honour-if-and-unless-options branch January 5, 2022 23:48
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.

3 participants