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

Stream configuration namespace cleanup. #5923

Merged

Conversation

jason-bragg
Copy link
Contributor

Inconsistent name spaces harm configuration discoverability.

  • Move all configurators in Orleans.Streams namespace to Orleans.Hosting
  • Move all configurators in Orleans.Configuration namespace to Orleans.Hosting
  • Added developer extension functions for EventDataGeneratorStream
  • Cleaned up naming issues with Memory Stream Configurator.

Inconsistent name spaces harm configuration discoverability.
- Move all configurators in Orleans.Streams namespace to Orleans.Hosting
- Move all configurators in Orleans.Configuration namespace to Orleans.Hosting
- Added developer extension functions for EventDataGeneratorStream
- Cleaned up naming issues with Memory Stream Configurator.
@@ -0,0 +1,37 @@
using System;

namespace Orleans.Hosting.Developer
Copy link
Member

Choose a reason for hiding this comment

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

I feel these should still be in Orleans.Hosting - the .Developer ns will make them very difficult to discover

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Understood. These are not surfaces intended for production use. They exist for developers and test. Being less discoverable was the intention. So I suspect the question we need resolve here is whether they should be less discoverable?

Copy link
Member

@ReubenBond ReubenBond Sep 3, 2019

Choose a reason for hiding this comment

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

I'm not sure. It's a hard question. A part of me wonders: should we be preventing users from using Dev-only functionality in Prod by checking some config flag? Eg, something like IHostEnvironment.IsDevelopment, or maybe something more Orleans-specific?

That's not an immediate solution... but in the short term maybe we could fix this with regular naming. My feeling is that we should make the functionality we offer discoverable but I don't know the use-case for generators in order to decide what value they have: if they are just for dev/test then I'm happy for no one to know about them (and even happier for us to delete them).

Copy link
Member

@ReubenBond ReubenBond left a comment

Choose a reason for hiding this comment

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

LGTM but one open question about .Developer namespace

@ReubenBond ReubenBond merged commit f3edb8b into dotnet:master Sep 3, 2019
@github-actions github-actions bot locked and limited conversation to collaborators Dec 2, 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

2 participants