-
Notifications
You must be signed in to change notification settings - Fork 483
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
Distinguish case sensitivity for NuGet.Config keys #3260
Conversation
Learn Build status updates of commit 7d73d00: ✅ Validation status: passed
For more details, please refer to the build report. For any questions, please:
|
@@ -47,7 +47,7 @@ A `NuGet.Config` file is a simple XML text file containing key/value pairs as de | |||
Settings are managed using the NuGet CLI [config command](../reference/cli-reference/cli-ref-config.md): | |||
- By default, changes are made to the user-level config file. (On Mac/Linux, the location of user-level config file varies by tooling) | |||
- To change settings in a different file, use the `-configFile` switch. In this case files can use any filename. | |||
- Keys are always case sensitive. | |||
- Keys are case sensitive when being referenced. New keys must be unique regardless of casing (case insensitive) . |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This sounds like a developer guide instead of a user facing doc.
I think you can tweak the phrasing to communicate the same thing in a slightly different way.
- Keys are case sensitive when being referenced. New keys must be unique regardless of casing (case insensitive) . | |
Keys are case sensitive, but they always unique regardless of casing. |
You don't have to accept my suggestion, but just an idea that I found more in style with the rest of the doc.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't find that more clear, so I'll let others chime in.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's fair to question whether my proposal is good enough, but New keys must be unique regardless of casing (case insensitive) .
currently is aimed the nuget devs since we're the only ones that can add new keys and we're not the audience for this doc.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Who is the target audience for this document? I thought if a customer is consuming packages, that means they are modifying nuget.configs, which they need to know the 2 points:
- that it matters what casing they use for their keys
- once a casing is created, another casing for the same key cannot be used
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
<packageSources>
<add key="Contoso Package Source" value="https://contoso.com/packages/" />
<add key="nuget.org" value="https://api.nuget.org/v3/index.json" />
</packageSources>
Here's a sample from the document. We need to say that this is invalid for a customer to add:
<add key="CONTOSO Package Source" value="https://contoso123.com/packages/" />
and that this is wrong given the above sample:
<disabledPackageSources>
<add key="NUGET.org" value="true" />
</disabledPackageSources>
and
<packageSourceMapping>
<clear />
<packageSource key="nuGeT.OrG">
<package pattern="Azure.Core" />
</packageSource>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the doc you are changing is talking about the <config>
section of the nuget.config file.
Keys such as globalPackagesFolder
etc, not the sources.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now that I read more of it, it never mentions keys such as sources/package source mapping, while it probably should, or at least link to a page that does.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For what it's worth, I find it all a bit confusing, and I've been working on NuGet for years 😕
This PR's proposal "Keys are case sensitive when being referenced", I'm not really sure what "being refereced" is referring to. Does this mean if I have <add key="globalpackagesfolder" value="d:\nuget\packages" />
the setting will be silently ignored, because it must be globalPackagesFolder
? If so, that sounds like a product bug that we should change, rather than document.
Same for dotnet push my.nupkg -s mysource
. If the config file has it defined as MySource
, are the docs telling me this will fail? Again, to me that sounds like a product bug we should fix, rather than document.
Donnie has a comment in this thread saying that customers adding new sources need to make the keys unique (case insensitive), which I can understand. What I don't understand are the scenarios when they are case sensitive, and whether it makes sense for our product to treat them as case sensitive or if it feel like a bug from a customer experience point of view.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For "being referenced", I gave 2 examples here #3260 (comment)
As far as making product changes, that should be a NuGet/Home issue, not a docs issue. I'm just trying to make the docs point out what the product does now, not what makes sense to change (which would be another docs change once the product change is released).
"always" case sensitive is misleading, as it makes it sound like adding a new key with a different case should be acceptable.
My goal is to clarify that keys are referenced in a case-sensitive manner.
Adding "nuget.org" and "nuGet.org" though should be denied by NuGet tooling as they are not unique (case-insensitive).