-
Notifications
You must be signed in to change notification settings - Fork 1
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
[CAT-323] Remove references to "load" #119
Conversation
Total Coverage: 33.01% Coverage Report
|
@@ -29,6 +29,8 @@ internal class IndicoTestContainerBuilder | |||
|
|||
public IUnityContainer Build() | |||
{ | |||
Console.WriteLine(BaseUrl); |
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.
please remove these console logs
/// <param name="modelId"><seealso cref="IModel"/>'s Id</param> | ||
/// <param name="cancellationToken"><c><see cref="CancellationToken"/></c> for handling cancellation of asynchronous operations.</param> | ||
/// <returns>Status</returns> | ||
Task<string> LoadModel(int modelId, CancellationToken cancellationToken); |
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.
I know this is more work, but I would like to mark this as @ deprecated and make the underlying call return a no-op/success by default. We don't want to delete API calls without first marking them deprecated so users can migrate.
I don't think this was completely necessary because the ModelGroupPredict integration tests were passing without it. But if we just want to clean up the repo a bit then this is a good move