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.

Sign up for GitHub

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

Jump to bottom

revert: "revert: "fix(flags): Do the expected for numeric comparisons"" #18415

Merged
merged 3 commits into from
Nov 7, 2023
Merged

revert: "revert: "fix(flags): Do the expected for numeric comparisons"" #18415

merged 3 commits into from
Nov 7, 2023

Conversation

neilkakkar
Copy link
Collaborator

@neilkakkar neilkakkar commented Nov 6, 2023 •
edited
Loading

Reverts #18413

Now with more fixes for things that broke:

  1. I didn't consider cohorts might have numeric properties too (d'oh!!), so we needed to add annotations for these props too, and sadly no existing tests around this to catch the problem. Fixed this, added tests.
  2. Some older clients had malformed props, so the "is not an array" check was breaking for them. Have restored old behaviour here now.
  3. None was a valid property value, and operating > , < blew up, so handling this case as well now.
image

For the diff from previous just check the latest commit

@neilkakkar neilkakkar changed the title revert "revert: "fix(flags): Do the expected for numeric comparisons"" revert: "revert: "fix(flags): Do the expected for numeric comparisons"" Nov 6, 2023
@neilkakkar neilkakkar marked this pull request as ready for review November 6, 2023 15:39
@neilkakkar neilkakkar merged commit 7148f03 into master Nov 7, 2023
64 checks passed
@neilkakkar neilkakkar deleted the revert-18413-revert-18359-flag-numeric branch November 7, 2023 12:07
neilkakkar added a commit that referenced this pull request Nov 7, 2023
@neilkakkar
Revert "revert: "revert: "fix(flags): Do the expected for numeric com…
206110a 
…parisons"" (#18415)"

This reverts commit 7148f03.
@neilkakkar neilkakkar mentioned this pull request Nov 7, 2023
Merged
Twixes pushed a commit that referenced this pull request Nov 7, 2023
@neilkakkar
revert: "revert: "revert: "fix(flags): Do the expected for numeric co…
0bdbd25 
…mparisons""" (#18441)

Revert "revert: "revert: "fix(flags): Do the expected for numeric comparisons"" (#18415)"

This reverts commit 7148f03.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@liyiy liyiy liyiy approved these changes

@jurajmajerik jurajmajerik jurajmajerik approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@neilkakkar @jurajmajerik @liyiy