这是indexloc提供的服务,不要输入任何密码
Skip to content

Conversation

@akshita31
Copy link
Contributor

@akshita31 akshita31 commented Sep 5, 2019

We would like to publish the Microsoft.Dotnet.Interactive and Microsoft.Dotnet.Interactive.Rendering packages so that it is possible to write extensions to the kernel.

This PR:

  1. Removes the dependency on MLS.Agent.Tools and publishes Microsoft.Dotnet.Interactive and Microsoft.DotNet.Interactive.Rendering nuget packages
  2. Removes clockwise dependency from the MLS.Agent.Tools
  3. For the Microsoft.Dotnet.Try.Markdown package it publishes the package with the MLS.Agent.Tools as a dll inside it so that we dont have to publish it separately as a nuget package

Copy link
Member

@colombod colombod left a comment

Choose a reason for hiding this comment

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

can we rename / address the ml.agent.tools? I don't think is ideal to ship / package like this

Copy link
Contributor

@jonsequitur jonsequitur left a comment

Choose a reason for hiding this comment

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

Let's take a look at how to avoid publishing a package for MLS.Agent.Tools.

@akshita31
Copy link
Contributor Author

The latest commit d9fd5ef adds a "beta" label for the library nuget packages and maintains the current behavior for the tool.

@akshita31
Copy link
Contributor Author

akshita31 commented Sep 11, 2019

Many thanks to @brettfo for helping to demistify the arcade stuff🥇

<PackageReference Include="Newtonsoft.Json" Version="12.0.02" />
<PackageReference Include="PocketLogger" Version="0.3.0">
<!-- Any new dependency added here must also be included in the Microsoft.DotNet.Try.Markdown, Microsoft.DotNet.Interactive and Microsoft.DotNet.Interactive.Rendering nuspec files -->
<PackageReference Include="Clockwise" Version="$(ClockwiseVersion)" />
Copy link

Choose a reason for hiding this comment

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

I'm really not a fan of this--we're doing something that explicitly makes things weirder and harder in the future (I acknowledge it was not your idea).

Copy link
Contributor

Choose a reason for hiding this comment

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

We should actually look to remove the Clockwise dependency from here.

@rchande
Copy link

rchande commented Sep 13, 2019

@jonsequitur Why don't we want to ship an MLS.Agent.Tools nuget package? The types within it are going to be public APIs regardless of what nuget package they ultimately appear in?

Copy link
Contributor

@jonsequitur jonsequitur left a comment

Choose a reason for hiding this comment

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

Let's try to remove the Clockwise dependency from the lower-level components. Individual recipes that depend on it would be better off moved up into MLS.Agent. A low-level utility library like MLS.Agent.Tools shouldn't bring along its own dependencies because then they spread everywhere.

@akshita31
Copy link
Contributor Author

In the latest commit, I have removed the dependency of Microsoft.Dotnet.Interactive on MLS.Agent.Tools

}

public async Task LoadExtensionsInDirectory(IDirectoryAccessor directory, KernelInvocationContext context)
public async Task LoadExtensionsInDirectory(DirectoryInfo directory, KernelInvocationContext context)
Copy link
Contributor

Choose a reason for hiding this comment

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

Since this doesn't rely directly on private state, this is a good candidate for an extension method.

Is this code C# specific?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This method is needed for implementing the interface IExtensibleKernel

@akshita31 akshita31 dismissed stale reviews from jonsequitur and colombod September 17, 2019 22:03

As discussed the MLS.Agent.Tools dll is a part of the Microsoft.DotNet.Interactive

@akshita31 akshita31 merged commit cdf3c33 into dotnet:master Sep 17, 2019
@akshita31 akshita31 deleted the publis branch September 17, 2019 22:59
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.

5 participants