8000 Try to fix the compare/2 implementations by adonig · Pull Request #1232 · ash-project/ash · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Try to fix the compare/2 implementations #1232

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
Merged
merged 1 commit into from
Jun 10, 2024
Merged

Try to fix the compare/2 implementations #1232

merged 1 commit into from
Jun 10, 2024

Conversation

adonig
Copy link
Contributor
@adonig adonig commented Jun 8, 2024

Contributor checklist

  • Bug fixes include regression tests
  • Chores
  • Documentation changes
  • Features include unit/acceptance tests
  • Refactoring
  • Update dependencies

@zachdaniel
Copy link
Contributor

There are some good changes in here, but another interesting bit is that compare should actually be defined automatically if bulk_compare is defined and should call into bulk_compare. I'll merge this and make those changes.

@zachdaniel zachdaniel merged commit 4153ba7 into ash-project:main Jun 10, 2024
30 checks passed
@zachdaniel
Copy link
Contributor

🚀 Thank you for your contribution! 🚀

@zachdaniel
Copy link
Contributor
zachdaniel commented Jun 10, 2024 •
edited
Loading

Nevermind, my changes don't really work. bulk_compare returns boolean statements and compare returns a "comparison". For now we'll leave it as is although in the future this should probably be redesigned (not your changes, I mean the existing code)

Sorry, something went wrong.

@adonig adonig deleted the fix/filter-predicate-compare-eq branch June 10, 2024 13:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@adonig @zachdaniel
0