Skip to content

Added Microsoft.Extensions.Features Project #31746

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

Closed
wants to merge 3 commits into from
Closed

Added Microsoft.Extensions.Features Project #31746

wants to merge 3 commits into from

Conversation

ShreyasJejurkar
Copy link
Contributor

@ShreyasJejurkar ShreyasJejurkar commented Apr 13, 2021

Tasklist

  • Create a new project Microsoft.Extensions.Features (netstandard2.0)
  • Move IFeatureCollection, FeatureCollection etc to this assembly. Add type-forwarding from Http.Features -> IFeatureCollection.
  • SignalR will reference this new project. This is a breaking change for SignalR.
  • Update Microsoft.AspNetCore.Http.Features to target net6.0 and stop publishing it as a NuGet package.

As part of #31723
cc @pranavkm @davidfowl

1. Added initial structure of Microsoft.Extensions.Features project.
2. Updated `HttpAbstractions` solution filter and AspNetCore.sln root solution to include new project.

As part of #31723
@ghost ghost added the community-contribution Indicates that the PR has been added by a community member label Apr 13, 2021
@davidfowl
Copy link
Member

@MCCshreyas this is one I'd want the team to do to avoid any lag or back and forth. Thanks for your efforts here!

@ShreyasJejurkar
Copy link
Contributor Author

Will it be ok, if I proceed with a task that @pranavkm mentioned, my target is to finish this one today itself. If I face any difficulties, and look like obstacles I will close this PR then!

But if everything goes correct and if I managed to get this all done today itself, so should I still close my PR, OR maybe someone from the team can proceed with my changes if those changes are hard and need some internal discussion before going.

@davidfowl
Copy link
Member

This one is tricky because we need to spend time dealing with the implications of the breaking change and messing with type forwards. It's a little intricate and not a good pick for a contribution

@ShreyasJejurkar
Copy link
Contributor Author

@davidfowl Let me try! 😄

1. Moved `IFeatureCollection`, `FeatureCollection` to new assembly.
2. Added `TypeForwardFrom` and `TypeForwardTo` Attribute for those types.
3. Added entries in ProjectReferences.props and SharedFramework.Local.props based on ReferenceResolution.md document.
4. Updated PublicAPI files for both project to reflect new changes.
@ShreyasJejurkar
Copy link
Contributor Author

ShreyasJejurkar commented Apr 13, 2021

  1. .dotnet/sdk/6.0.100-preview.3.21168.19/Sdks/Microsoft.NET.Sdk/targets/Microsoft.PackageDependencyResolution.targets(241,5): error NETSDK1005: (NETCORE_ENGINEERING_TELEMETRY=Build) Assets file '/__w/1/s/artifacts/obj/Microsoft.Extensions.Features/project.assets.json' doesn't have a target for 'netstandard2.0'. Ensure that restore has run and that you have included 'netstandard2.0' in the TargetFrameworks for your project
    Saw this error in the build pipeline! But the new project is already targeting netstandard2.0, seems like I missed making an entry somewhere, but not sure!

    <PropertyGroup>
    <TargetFramework>netstandard2.0</TargetFramework>
    </PropertyGroup>

  2. ##[error].dotnet\sdk\6.0.100-preview.3.21168.19\Sdks\NuGet.Build.Tasks.Pack\build\NuGet.Build.Tasks.Pack.targets(221,5): error NU5026: (NETCORE_ENGINEERING_TELEMETRY=Build) The file 'D:\workspace\_work\1\s\artifacts\bin\Microsoft.Extensions.Features\Release\net461\Microsoft.Extensions.Features.dll' to be packed was not found on disk.
    What needs to be done to make it packable? I saw the following on some projects file
    <IsPackable>false</IsPackable>
    Do I need to add the same for the new project with true as value?

Help on this would be appreciated. 😄

@davidfowl
Copy link
Member

I'd rather not. Can you take on another issue please. As I said before this one requires a bit more care and it would be better for the team to handle it. It's not that it couldn't be done by yourself, but we didn't mark it "help wanted" for a very specific reason.

@ShreyasJejurkar
Copy link
Contributor Author

Ummm. I keep those issues for other people who are new to the repo and or the first time. I took this issue because based on the issue description, it was looking not very hard. But as you suggested, these changes need to be discussed internally before proceed, so I will close this PR as of now!

But later if you guys, found I was on the correct track of changes and also need some more changes, then please feel free to re-open this PR, I like to continue from there as well!

Thanks, 😑😊

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
community-contribution Indicates that the PR has been added by a community member
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants