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

Updating EventHubAdapterFactory for extensibility. #1861

Merged
merged 2 commits into from
Jun 24, 2016
Merged

Updating EventHubAdapterFactory for extensibility. #1861

merged 2 commits into from
Jun 24, 2016

Conversation

kylemc
Copy link
Contributor

@kylemc kylemc commented Jun 20, 2016

  • Making the provider config a protected member that can be accessed in derived factory types
  • Making the QueueMapper public so custom implementations can be authored in derived types

- Making the provider config a protected member that can be accessed in derived factory types
- Making the QueueMapper public so custom implementations can be authored in derived types
@dnfclas
Copy link

dnfclas commented Jun 20, 2016

Hi @kylemc, I'm your friendly neighborhood .NET Foundation Pull Request Bot (You can call me DNFBOT). Thanks for your contribution!
You've already signed the contribution license agreement. Thanks!

The agreement was validated by .NET Foundation and real humans are currently evaluating your PR.

TTYL, DNFBOT;

@jason-bragg
Copy link
Contributor

Looks good.
Only thing that caught my eye was that the Queue mapper factory is asynchronous. Queue mappers have historically been light weight mapping logic with no external dependencies. I'm curious why an asynchronous factory is needed.
My preference would be to constrain the creation of the mapper to a synchronous call, if possible.

@kylemc
Copy link
Contributor Author

kylemc commented Jun 21, 2016

I'm happy either way. I was just following the pattern of all the other factories in the adapter, but I can make this one synchronous.

@jason-bragg jason-bragg merged commit 16b7b59 into dotnet:master Jun 24, 2016
@jason-bragg
Copy link
Contributor

Thanks @kylemc

@github-actions github-actions bot locked and limited conversation to collaborators Dec 12, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants