-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
Need to rename TlcEnvironment #541
Comments
ConsoleEnvironment? |
I had a similar thought, but then I started looking at this and found it does a lot more than just logging which made it hard for me to name its purpose. I seem to remember some discussions before about how we might want to eliminate the environment abstraction completely, is that still an option? I see it exposes stuff like cancellation that seems to only be partially implemented (cancellation tokens instead?), random number generation (System.Random?) and temporary files (can't it just be a utility?) etc etc. Some of this stuff might still be useful for testing, but right now this abstraction is front and center in the samples and all over the APIs: seems like the wrong tradeoff to put this complexity on all callers just to facilitate the small set of folks who might want to provide a different environment. |
I somewhat disagree with the "Need" to rename this, but perhaps the only option that I can think of is |
Once you ship an API (especially one that is core to every scenario, like the environment class is setting itself up to be), you can never remove, rename, change it. You can only made additional, non-breaking changes. So in a few years, if all our examples have |
Some more proposals that were floated are 'LocalEnvironment', 'LocalConsoleEnvironment', 'Environment' and 'MlContext'. |
Fixed by #923 |
Our only public, concrete environment class is named "TlcEnvironment", which uses the internal "TLC" acronym/name and has no meaning anymore.
We should come up with a better name for our default environment class.
/cc @ericstj @TomFinley
The text was updated successfully, but these errors were encountered: