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.

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

Add CPM scenario for NU1604 #3320

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Add CPM scenario for NU1604 #3320

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 19 additions & 0 deletions docs/reference/errors-and-warnings/NU1604.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,3 +32,22 @@ or
> `<PackageReference Version="9.0.0" />`

which implies a lower bound.

If using [Central Package Managment](https://learn.microsoft.com/nuget/consume-packages/Central-Package-Management) you will need to update the `PackageVersion` `Version` attribute in your `Directory.Packages.Props` file.
Copy link
Member

Choose a reason for hiding this comment

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

Normally this would be a different scenario.

We'd have another Issue, Solution etc.

We should also call out that this is a special case, since in a properly onboarded CPM scenario, you'd get NU1008.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I wasn't sure if I should put it in another Solution section since it's the same error but yeah it's a special case so I can start with that

Copy link
Member

@nkolev92 nkolev92 Jul 22, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Here's prior art from both restore and pack.

Copy link
Member

Choose a reason for hiding this comment

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

When CPM is enabled, is the warning/error message the same, or different?

If the warning/error message is different when CPM is enabled compared to when it's not, it's important to have a sample of the message in these docs so that if the customer searches those words, they're more likely to find these docs.

Copy link
Contributor

Choose a reason for hiding this comment

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

nit: my readability preference would be for something between these two words..

Suggested change
If using [Central Package Managment](https://learn.microsoft.com/nuget/consume-packages/Central-Package-Management) you will need to update the `PackageVersion` `Version` attribute in your `Directory.Packages.Props` file.
If using [Central Package Managment](https://learn.microsoft.com/nuget/consume-packages/Central-Package-Management) you will need to update the `PackageVersion` element's `Version` attribute in your `Directory.Packages.Props` file.

Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
If using [Central Package Managment](https://learn.microsoft.com/nuget/consume-packages/Central-Package-Management) you will need to update the `PackageVersion` `Version` attribute in your `Directory.Packages.Props` file.
If using [Central Package Managment](https://learn.microsoft.com/nuget/consume-packages/Central-Package-Management), you will need to update the `PackageVersion` `Version` attribute in your `Directory.Packages.Props` file.

For example change from:
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
For example change from:
For example, change from:


> `<PackageVersion Include="PackageA" Version="(9.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.

extra blank line

to:

> `<PackageVersion Include="PackageA" Version="[9.0.0, )" />`

or

> `<PackageVersion Include="PackageA" Version="9.0.0" />`

which implies a lower bound.

> [!Note]
> When using CPM and the file `Directory.Packages.Props` is invalid, NU1604 is raised.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added this because the original issue is related to an invalid file but makes me think this is a bug instead of a docs issue. @jeffkl what do you think?