Skip to content
This repository was archived by the owner on Nov 6, 2018. It is now read-only.

Consider EmbeddedFileSystem becoming its own NuGet package #100

Merged
merged 1 commit into from
Apr 23, 2015
Merged

Conversation

Praburaj
Copy link
Contributor

@ghost ghost added the cla-already-signed label Apr 21, 2015
{
"version": "1.0.0-*",
"description": "Implementation of ASP.NET 5 physical file provider abstractions.",
"dependencies": {
Copy link
Member

Choose a reason for hiding this comment

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

Don't you need a dependency on Interfaces here? Common gets you one transitively, but it's only a build-time dependency. Same for Embedded.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Right I see that. Otherwise ones consuming these packages will have to include the file system interfaces in their projects.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated.

@Tratcher
Copy link
Member

The code looks fine, though I'm not sure what is accomplished by fragmenting to this degree.

@Praburaj
Copy link
Contributor Author

This was out of the API review. My understanding is when user wants PhysicalFileSystem then we don't need to pull in Embedded one. Just a matter of organizing.

"description": "Implementation of ASP.NET 5 file provider abstractions.",
"dependencies": {
"Microsoft.AspNet.FileProviders.Interfaces": "1.0.0-*"
},
Copy link
Contributor

Choose a reason for hiding this comment

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

Don't shared source projects need "shared": "*.cs", ? Or is this not shared source? The bug didn't say anything about having a package named "Microsoft.AspNet.FileProviders.Common" did it?

Copy link
Member

Choose a reason for hiding this comment

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

Fixed.

@Tratcher
Copy link
Member

Updated @Eilon


using System.Reflection;

[assembly: AssemblyMetadata("Serviceable", "True")]
Copy link
Contributor

Choose a reason for hiding this comment

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

This package doesn't really produce an assembly right? If no assembly, don't need this attribute.

@Tratcher
Copy link
Member

Updated.

@Eilon
Copy link
Contributor

Eilon commented Apr 23, 2015

:shipit:

@Tratcher Tratcher merged commit bf092bc into dev Apr 23, 2015
@Tratcher Tratcher deleted the bug48 branch April 23, 2015 18:49
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants