-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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: refactor filters into standalone package #6182
feat: refactor filters into standalone package #6182
Conversation
6344a88
to
091407e
Compare
ef470fd
to
7bbebea
Compare
The coverage drop doesn't seem to be related to this PR. |
Thanks for the feedback, I agree with the points and applied them; PTAL. |
@@ -0,0 +1,135 @@ | |||
# @loopback/filter |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Which name is better?
@loopback/filter
@loopback/query-filter
@loopback/repository-filter
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I prefer @loopback/filter
as the filter is use both when working with queries and repositories. Prefixing it with query-
or repository-
may create confusion that the filter only applies when working with either one of those concepts.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good discussion!
I vote for @loopback/filter
.
@achrinza Good initiative. The change LGTM. Let's settle on the name - https://github.com/strongloop/loopback-next/pull/6182/files#r477425945. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great initiative, @achrinza!
The code looks mostly good, I have one comment to address 👇🏻
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
3979d5a
to
794d8b8
Compare
see: loopbackio#5957 Signed-off-by: Rifa Achrinza <[email protected]>
794d8b8
to
dd012b3
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👏🏻
see: #5957
Signed-off-by: Rifa Achrinza [email protected]
Checklist
npm test
passes on your machinepackages/cli
were updatedexamples/*
were updated👉 Check out how to submit a PR 👈