Skip to content

Ensure Microsoft.AspNetCore.Razor.RuntimeCompilation has the right build targets #14863

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Oct 11, 2019
Merged

Ensure Microsoft.AspNetCore.Razor.RuntimeCompilation has the right build targets #14863

merged 1 commit into from
Oct 11, 2019

Conversation

pranavkm
Copy link
Contributor

@pranavkm pranavkm commented Oct 9, 2019

  • Include build and build transitive folders in RuntimeCompilation package
  • Use a different folder name to allow GitHub indexing. See Rename "build" directories #14846
  • Add a test to verify transitive build targets work

Fixes #14813
Fixes #12768

@pranavkm
Ensure Microsoft.AspNetCore.Razor.RuntimeCompilation has the right bu…
6aaf694 
…ild targets

* Include build and build transitive folders in RuntimeCompilation package
* Use a different folder name to allow GitHub indexing. See #14846
* Add a test to verify transitive build targets work

Fixes #14813
Fixes #12768
@pranavkm pranavkm requested a review from ajaybhargavb October 9, 2019 21:47
ajaybhargavb
ajaybhargavb reviewed Oct 9, 2019
View reviewed changes
...vc/Mvc.Razor.RuntimeCompilation/src/Microsoft.AspNetCore.Mvc.Razor.RuntimeCompilation.csproj
@@ -23,6 +23,7 @@
</ItemGroup>

<ItemGroup>
<None Include="build\$(DefaultNetCoreTargetFramework)\*" Pack="true" PackagePath="build\$(DefaultNetCoreTargetFramework)" />
<None Include="targets\$(PackageId).targets" Pack="true" PackagePath="build\$(DefaultNetCoreTargetFramework)\$(PackageId).targets" />
Copy link
Contributor

Choose a reason for hiding this comment

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

Who sets packageid? Also, are we renaming the folder name to netcoreapp3.1?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

PackageId is set by the SDK. So this should result in

<None Include="targets\Microsoft.AspNetCore.Mvc.Razor.RuntimeCompilation.targets" Pack="true" PackagePath="build\netcoreapp3.1\Microsoft.AspNetCore.Mvc.Razor.RuntimeCompilation.targets" />

@mkArtakMSFT mkArtakMSFT added the area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates label Oct 9, 2019
ajaybhargavb
ajaybhargavb approved these changes Oct 11, 2019
View reviewed changes
src/Mvc/test/WebSites/RazorBuildWebSite/RazorBuildWebSite.csproj
@@ -21,4 +20,6 @@
<Reference Include="Microsoft.AspNetCore.Diagnostics" />
</ItemGroup>

<Import Project="..\..\..\Mvc.Razor.RuntimeCompilation\src\targets\Microsoft.AspNetCore.Mvc.Razor.RuntimeCompilation.targets" />
Copy link
Contributor

Choose a reason for hiding this comment

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

No $(SolutionRoot)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ehh, unless something like helix requires it

@pranavkm pranavkm merged commit 6dee2f5 into release/3.1 Oct 11, 2019
@pranavkm pranavkm deleted the prkrishn/build-transitive branch October 11, 2019 20:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ajaybhargavb ajaybhargavb ajaybhargavb approved these changes

@ryanbrandenburg ryanbrandenburg Awaiting requested review from ryanbrandenburg

Assignees
No one assigned
Labels
area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@pranavkm @ajaybhargavb @mkArtakMSFT