Skip to content
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

[Behavior Change] Raise a missing README warning to <NuGetRequireReadMe> is true #13765

Closed
JonDouglas opened this issue Sep 4, 2024 · 10 comments
Labels
Functionality:Pack Resolution:Duplicate This issue appears to be a Duplicate of another issue Type:DCR Design Change Request

Comments

@JonDouglas
Copy link
Contributor

NuGet Product(s) Affected

NuGet SDK

Current Behavior

When packing a NuGet project, we warn if a README is missing.

Desired Behavior

When packing a NuGet project and a project specifies , we should upgrade to an error if a README is missing.

Additional Context

https://learn.microsoft.com/en-us/nuget/reference/errors-and-warnings/nu5039

@JonDouglas JonDouglas added Type:DCR Design Change Request Triage:Untriaged labels Sep 4, 2024
@zivkan
Copy link
Member

zivkan commented Sep 5, 2024

I think it would be better if we made pack warnings understand .editorconfig files, and allow customers to customize pack messages in the same way they can roslyn analyzer messages.

@baronfel
Copy link

baronfel commented Sep 5, 2024

I think it would be better if we made pack warnings understand .editorconfig files, and allow customers to customize pack messages in the same way they can roslyn analyzer messages.

Good news! This is something you get for free if you implement the pack warnings as MSBuild BuildChecks! Editorconfig-based configuration is a core aspect of the BuildCheck model.

@kartheekp-ms kartheekp-ms added Functionality:Pack Product:NuGet.exe NuGet.exe Product:dotnet.exe Functionality:SDK The NuGet client packages published to nuget.org and removed Triage:Untriaged labels Sep 6, 2024
@nkolev92
Copy link
Member

Which warning are we referring to here?
NU5039 is an error that's only raised when the readme is attempted to be packed, but not succesfully.

The other part is not a warning, but an info message.

@nkolev92 nkolev92 added Triage:NeedsMoreInfo WaitingForCustomer Applied when a NuGet triage person needs more info from the OP labels Sep 23, 2024
@zivkan
Copy link
Member

zivkan commented Sep 23, 2024

@nkolev92 the title of this issue is to change the info message to a warning.

Adding a NuGetRequireReadMe property is not necessary, because once it's a warning, and has an NU code, customers can <WarningsAsError> the code, or if we go down the editorconfig route (or MSBuild BuildChecks), then set it in their editorconfig file.

@nkolev92
Copy link
Member

@nkolev92 the title of this issue is to change the info message to a warning.

It's not, which is why I asked for a clarification, it's referring to a current warning, but It's also linking NU5039, which if it's about upgrading the message to a warning, NU5039 is not really relevant.

@JonDouglas
Copy link
Contributor Author

Which warning are we referring to here? NU5039 is an error that's only raised when the readme is attempted to be packed, but not succesfully.

The other part is not a warning, but an info message.

We need a warning code for missing README. I thought this we did this but I guess we didn't? #12070

@dotnet-policy-service dotnet-policy-service bot added WaitingForClientTeam Customer replied, needs attention from client team. Do not apply this label manually. and removed WaitingForCustomer Applied when a NuGet triage person needs more info from the OP labels Sep 23, 2024
@nkolev92 nkolev92 changed the title [Behavior Change] Upgrade missing README warning to and error if <NuGetRequireReadMe> is true [Behavior Change] Raise a missing README warning to <NuGetRequireReadMe> is true Sep 23, 2024
@nkolev92 nkolev92 removed WaitingForClientTeam Customer replied, needs attention from client team. Do not apply this label manually. Triage:NeedsMoreInfo Functionality:SDK The NuGet client packages published to nuget.org Product:NuGet.exe NuGet.exe Product:dotnet.exe labels Sep 23, 2024
@nkolev92
Copy link
Member

Thanks for the clarifications. Fixed the title, this is ready for the next triage.

@nkolev92
Copy link
Member

Dup of #13633.

@nkolev92 nkolev92 closed this as not planned Won't fix, can't repro, duplicate, stale Sep 23, 2024
@nkolev92 nkolev92 added the Resolution:Duplicate This issue appears to be a Duplicate of another issue label Sep 23, 2024
@JonDouglas
Copy link
Contributor Author

The request for the standalone property was because we don't have this coded either approach should work.

@nkolev92
Copy link
Member

The request for the standalone property was because we don't have this coded either approach should work.

Yes, it's just consolidating the asks.
The ideal solution would be one that scales as @zivkan and @baronfel were discussing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Functionality:Pack Resolution:Duplicate This issue appears to be a Duplicate of another issue Type:DCR Design Change Request
Projects
None yet
Development

No branches or pull requests

5 participants