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

Feature/ Make SelectExpandBinder and AggregationBinder public and let override there methods #1921

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

savelievser
Copy link

@savelievser savelievser commented Oct 3, 2019

Issues

This pull request fixes issue implements feature #1900 .

Description

SelectExpandBinder and AggregationBinder were made public and some of there methods were made protected virtual to let developers override it's functionality (like in FilterBinder). Creation of these classes were moved to new DataBinderProvider class, because approach with registering them with IServiceProvider will require big refactoring.

Checklist (Uncheck if it is not completed)

  • Build and test with one-click build and test script passed

Additional work necessary

I think no

@savelievser
Copy link
Author

How can I setup local environment to receive same errors as continuous-integration/vsts?
Locally run build.cmd executes successfully. Can somebody tell what errors are?

@KanishManuja-MS
Copy link
Contributor

How can I setup local environment to receive same errors as continuous-integration/vsts?
Locally run build.cmd executes successfully. Can somebody tell what errors are?

I know it is slow and painful but we will soon fix this issue which will enable you to see the build results. In the meanwhile, here is the output -

image

@savelievser
Copy link
Author

savelievser commented Oct 7, 2019

Thank you @KanishManuja-MS . Do I need to perform any actions to let your team start review process of this PR?

@savelievser
Copy link
Author

Hi @KanishManuja-MS!
Could you please provide some estimation of how long will it take to review this PR? In our project we are counting on this change and it would be great to understand in what library version it can be potentially included. Thanks!

@bcorkovic
Copy link

Hi @KanishManuja-MS

We are also interested into this PR. Is there any timeframe or option to help?

@dropsonic
Copy link

Any updates on this PR? It's been a year.

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

Successfully merging this pull request may close these issues.

4 participants