approvals / approvals/ApprovalTests.Ruby

Redifine match behaviour for filters?

Open
#36 1 comment 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
Ruby
Stars
231
Forks
45
PR merge metrics
No merged PRs in 30d

Description

I wonder why the choice was made to make a filter match regularly, instead of requiring it to match the whole string. Compare:

filters = {
  time: /^time$/,
  id: /(^|_)id$/,
  timestamp: /_at$/
}

versus

filters = {
  time: /time/,
  id: /(.*_)?id/,
  timestamp: /.*_at/
}

Imo the latter describe much clearer the content of the string matched, whereas the former sound more cryptical, and is easier to make mistakes with, like http://rubular.com/r/EnyxkU60Jo

Therefore I would be in favour of doing all-string matches always.

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start by locating the filter-matching implementation and its existing tests, then compare their behavior with the regex examples in the issue. Determine the intended whole-string matching semantics and compatibility impact before proposing a change. Done means the behavior is agreed, implemented, and covered by tests for the shown filter cases.

Written by the indexing model from the issue text.

Assessment

Tech stack
ruby
Domain
testing-qa
Issue type
Feature
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
25/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.