Skip to content

Tweak AddRazorPages trimming warning. #49377

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 3 commits into from
Aug 10, 2023
Merged

Tweak AddRazorPages trimming warning. #49377

merged 3 commits into from
Aug 10, 2023

Conversation

mitchdenny
Copy link
Member

Fixes #49363

@mitchdenny mitchdenny requested a review from a team as a code owner July 13, 2023 00:38
@mitchdenny mitchdenny self-assigned this Jul 13, 2023
@ghost ghost added area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates old-area-web-frameworks-do-not-use *DEPRECATED* This label is deprecated in favor of the area-mvc and area-minimal labels labels Jul 13, 2023
@mitchdenny mitchdenny requested a review from eerhardt July 18, 2023 02:21
eerhardt
eerhardt reviewed Jul 18, 2023
View reviewed changes
eerhardt
eerhardt reviewed Jul 18, 2023
View reviewed changes
@ghost
Copy link

ghost commented Jul 25, 2023

Looks like this PR hasn't been active for some time and the codebase could have been changed in the meantime.
To make sure no conflicting changes have occurred, please rerun validation before merging. You can do this by leaving an /azp run comment here (requires commit rights), or by simply closing and reopening.

@ghost ghost added the pending-ci-rerun When assigned to a PR indicates that the CI checks should be rerun label Jul 25, 2023
@DamianEdwards
Copy link
Member

@mitchdenny We going to merge this one?

@mitchdenny
Copy link
Member Author

@mitchdenny We going to merge this one?

It should go in but I don't think we have a good pre-existing place to point https://aka.ms/aspnet/trimming to.

@eerhardt
Copy link
Member

eerhardt commented Aug 2, 2023

I think we need to re-think all the attributes/messages that went in to #47663. Since we are using RequiresUnreferencedCode, we should also be saying "trimming or native AOT" in them all.

@DamianEdwards
Copy link
Member

I think we need to re-think all the attributes/messages that went in to #47663. Since we are using RequiresUnreferencedCode, we should also be saying "trimming or native AOT" in them all.

@mitchdenny Can we update this PR to do that?

@mitchdenny mitchdenny mentioned this pull request Aug 10, 2023
Open
@mitchdenny mitchdenny requested a review from eerhardt August 10, 2023 04:32
@mitchdenny mitchdenny force-pushed the tweak-razor-aot-trim-warning-message branch from 0efbf4c to 8836540 Compare August 10, 2023 04:32
@mitchdenny
Copy link
Member Author

@DamianEdwards / @eerhardt as discussed this morning about trim warnings. Would like to get this in for the upcoming RC.

@adityamandaleeka just in case we need extra approval for this (minor) change.

Note: I've setup the aka.ms link and filed a doc issue:

dotnet/AspNetCore.Docs#30018

eerhardt
eerhardt reviewed Aug 10, 2023
View reviewed changes
eerhardt
eerhardt approved these changes Aug 10, 2023
View reviewed changes
Copy link
Member

@eerhardt eerhardt left a comment

Choose a reason for hiding this comment

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

LGTM

@eerhardt eerhardt enabled auto-merge (squash) August 10, 2023 15:17
@eerhardt eerhardt merged commit 86c46c0 into dotnet:main Aug 10, 2023
@ghost ghost added this to the 8.0-rc1 milestone Aug 10, 2023
@eerhardt eerhardt mentioned this pull request Aug 29, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@eerhardt eerhardt eerhardt approved these changes

@BrennanConroy BrennanConroy Awaiting requested review from BrennanConroy BrennanConroy is a code owner

@halter73 halter73 Awaiting requested review from halter73 halter73 is a code owner

@Tratcher Tratcher Awaiting requested review from Tratcher

Assignees

@mitchdenny mitchdenny

Labels
area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates old-area-web-frameworks-do-not-use *DEPRECATED* This label is deprecated in favor of the area-mvc and area-minimal labels pending-ci-rerun When assigned to a PR indicates that the CI checks should be rerun
Projects
None yet
Milestone
8.0-rc1
Development

Successfully merging this pull request may close these issues.

Trimming warning is misleading
3 participants
@mitchdenny @DamianEdwards @eerhardt