-
-
Notifications
You must be signed in to change notification settings - Fork 65
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(scope): make Abstractions layer #131
base: master
Are you sure you want to change the base?
Conversation
PR Summary
|
Hi @mo3in, Soon I'll review your PR, meanwhile, it would be nice if you could also update the documentation, |
Just one more thing I can't decide on yet is that typically, abstraction packages also include extension methods. However, in the current version, we don't have them. I'd like to know your opinion on this. Do you think relocating interfaces and DTO classes would be sufficient? 🤔 (also feel free if you prefer to respond in Persian) |
Yes, you are right, but I believe there are additional issues. Upon reviewing
On the other hand, the abstraction layer should enable developers to provide custom implementations for Gridify. As of now, one approach could be to move DTOs/Interfaces to a project named Your assistance would be greatly appreciated... |
One approach we can take for now is to create the Abstraction package with a minimum set of features but plan to refactor the underlying services/extensions for v3 to support different implementations, (obviously with some breaking changes). As you mentioned I also want to support CursorPagination soon, so I think some refactoring is needed for sure. I believe having another Or we can hold this PR for now and try to add the Abstraction package in v3 after refactoring. Does the current Abstraction layer solve any problem for you? I don't see a real value yet (considering most of the Extensions and QueryBuilder class will remain in the main package) |
Hi @mo3in, |
@@ -0,0 +1,25 @@ | |||
<Project> |
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.
we can also use this props file, for Gridify.Elasticsearch package
Description
Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context.
Fixes #130 (issue)
Type of change
Checklist