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

Search for contacts and deals #37

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

protomolequle
Copy link

@protomolequle protomolequle commented Feb 17, 2025

Description

  • Search for contacts by email
  • Search for deals by name

Reviewers

@protomolequle protomolequle force-pushed the feat/contacts-and-deals branch 2 times, most recently from 6fe6584 to 50727e0 Compare February 17, 2025 03:44
@protomolequle protomolequle force-pushed the feat/contacts-and-deals branch from 50727e0 to cef044e Compare February 17, 2025 04:08
@kk-no kk-no self-requested a review February 17, 2025 06:39
@kk-no kk-no added the enhancement New feature or request label Feb 17, 2025
@kk-no
Copy link
Member

kk-no commented Feb 17, 2025

@protomolequle
Thank you for your understanding!
I will check the three PRs within this week, so please wait for a reply.

Copy link
Member

@kk-no kk-no left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@protomolequle
Thank you for adding this feature.
We would love to include this implementation in the library.

I would like to make it a little more general-purpose, allowing the user of the library to set things like Filter.
For example, the method name is simply Search, and the user of the library defines what to use for filtering.
I won't force you to go that far with this PR, but what do you think of the policy?

It would also be very helpful if you could add test code for the current code.
Thank you in advance.

@kk-no
Copy link
Member

kk-no commented Feb 20, 2025

Even if users specify filters, it may be better to predefine the ones that are likely to be used frequently, such as email and name.

@protomolequle
Copy link
Author

@kk-no, sure I can do that in a future PR. Making a more generalized search feature.

@kk-no
Copy link
Member

kk-no commented Feb 27, 2025

@protomolequle
Do you want to merge this PR once?

@protomolequle
Copy link
Author

@kk-no Yes, please do

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants