-
Notifications
You must be signed in to change notification settings - Fork 56
Correct filter models #19
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
Conversation
...r-block-object-apis Add support for Block API's 💖
Required to use actual serializer settings in tests
One test case for each filter type
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.
@asevos Does it make sense to move Condition classes as part of respective Filter class and rename them to just Condition. Then we can use them as [Filter].Condition?
Rest all LGTM.
@KoditkarVedant thank you for reviewing!
Yeah, moving conditions to be nested in filter class is a good idea, should've done that from the start. I will update my PR with this changes later today.
@asevos I've added editorconfig to the solution. you might wanna run the dotnet format.
@KoditkarVedant I moved conditions, formatted my files, added constructors to filters (as discussed in my constructor proposal ) and added to readme a section on how to query a db
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.
LGTM. Appreciate adding instructions on readme file.
I moved every filter to a separate file, created filter condition classes. All filters have correct format now.
All filter properties with default values were wrapped in
Nullable<T>.I also wrote some test cases for filters. For this I changed
RestClient.defaultSerializerSettingsaccess toprotected, so I could use in tests serializer settings identical to settings inRestClient