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

feat: rule.if #77

Merged
merged 5 commits into from
May 29, 2024
Merged

feat: rule.if #77

merged 5 commits into from
May 29, 2024

Conversation

hirasso
Copy link
Member

@hirasso hirasso commented May 29, 2024

Description

It's time! For a current project I finally need rule.if. This is a rewrite of #47

  • added a new test
  • also manually tested the behavior in the project where I need it

Questions

  • naming: rule.if or rule.predicate? Currently, the property is if, the type if Predicate.
  • currently, rule.if will prevent a rule from being matched if it returns a falsy value (if (!this.if)...). Would it make sense to make that more strict? (if (this.if === false))?

Drive-By

  • cleaned up some code comments

Checks

  • The PR is submitted to the master branch
  • The code was linted before pushing (npm run lint)
  • All tests are passing (npm run test)
  • New or updated tests are included
  • The documentation was updated as required

@hirasso hirasso requested a review from daun May 29, 2024 12:20
Copy link

github-actions bot commented May 29, 2024

Playwright test results

passed  12 passed

Details

stats  12 tests across 1 suite
duration  
commit  d973886

@daun
Copy link
Member

daun commented May 29, 2024

Yeah, been looking forward to this 🥯 Calling it if makes more sense to me than predicate, in terms of readability. Loose boolean matching is probably also fine -- as long as ommitting the if function is equivalent to () => true.

@hirasso
Copy link
Member Author

hirasso commented May 29, 2024

as long as ommitting the if function is equivalent to () => true

Yes, it is. ok great!

@hirasso hirasso merged commit 06f1594 into master May 29, 2024
3 checks passed
@hirasso hirasso deleted the feat/rule.if branch May 29, 2024 14:12
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.

2 participants