-
Notifications
You must be signed in to change notification settings - Fork 488
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
Add API docs for owners' template resource. #3282
base: main
Are you sure you want to change the base?
Conversation
Learn Build status updates of commit b8de56d:
|
File | Status | Preview URL | Details |
---|---|---|---|
docs/TOC.md | View | Details | |
docs/api/overview.md | ✅Succeeded | View | |
docs/api/owner-details-template-resource.md | ✅Succeeded | View |
docs/TOC.md
- Line 128, Column 6: [Warning: file-not-found - See documentation]
Invalid file link: 'api/package-owner-template-resource.md'.
For more details, please refer to the build report.
Note: Your PR may contain errors or warnings or suggestions unrelated to the files you changed. This happens when external dependencies like GitHub alias, Microsoft alias, cross repo links are updated. Please use these instructions to resolve them.
For any questions, please:
- Try searching the learn.microsoft.com contributor guides
- Post your question in the Learn support channel
@@ -0,0 +1,68 @@ | |||
--- |
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 doc LGTM.
Learn Build status updates of commit 7d33f49:
|
File | Status | Preview URL | Details |
---|---|---|---|
docs/TOC.md | View | Details | |
docs/api/overview.md | ✅Succeeded | View | |
docs/api/owner-details-template-resource.md | ✅Succeeded | View |
docs/TOC.md
- Line 126, Column 6: [Warning: file-not-found - See documentation]
Invalid file link: 'api/package-owner-template-resource.md'.
For more details, please refer to the build report.
Note: Your PR may contain errors or warnings or suggestions unrelated to the files you changed. This happens when external dependencies like GitHub alias, Microsoft alias, cross repo links are updated. Please use these instructions to resolve them.
For any questions, please:
- Try searching the learn.microsoft.com contributor guides
- Post your question in the Learn support channel
Learn Build status updates of commit d2d7f17: ✅ Validation status: passed
For more details, please refer to the build report. For any questions, please:
|
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.
Owner usernames are not asserted to be case insensitive ecosystem wide at this time.
## URL template | ||
|
||
The URL for the following API is the value of the `@id` property associated with one of the aforementioned | ||
resource `@type` values. |
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'm sorry, but I really don't understand what this sentence means, or what information this "URL template" section is trying to convey.
According to the docs style and voice guide: https://learn.microsoft.com/en-au/contribute/content/style-quick-start
the docs should be written in a casual voice, like a conversation, and use simple, easy to read sentences. I don't think this sentence/paragraph is doing that.
FWIW, I feel like this is a good place for the first example value. Reading the doc top down, it all feels a bit abstract and hard to pin down, until the examples in the last 3 lines of the docs. If there was an example here, early on in the docs page, I think it makes the rest of the docs more relatable.
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.
Tbh I just copied from other documents that have the same section. @joelverhagen I don't see a lot of value in having this section in the documentation, do you think we should keep it? If so, how can we improve this section?
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 feel strongly. Feel free to edit it according to @zivkan's suggestions. Sounds reasonable.
Learn Build status updates of commit 786f6e2: ✅ Validation status: passed
For more details, please refer to the build report. For any questions, please:
|
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.
Thanks for working on the docs and the template! I think some of the examples are for the package details template.
|
||
Name | Type | Required | Notes | ||
----------- | ------- | -------- | ----- | ||
`{owner}` | string | no | The owner ID to get details for |
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 hadn't considered an optional parameter. Is the idea that every owner link would actually just point to the Gallery?.
This may help package sources to temporarily implement this feature by pointing their owner details hyperlink to some landing page (like Gallery?).
/cc @joelverhagen for additional thoughts
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.
There are two kinds of required that matter I think: 1) the client rejects or errors out when no {owner}
parameter is found and 2) we say it's required in the docs because we think it should really be there, but we make no effort to enforce it in the client.
I think having the same URL for every owner (i.e. no {owner}
token) is a bad user experience but I don't know how to best surface that error in client.
In short, I think saying required = yes in the docs as a description of intent is okay even if client does no validation on this. required = no gives the wrong intent
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.
"Required yes" sounds good to me.
I agree that it's a bad user experience to have identical links for all owners, but we could leave that up to the package source. I wanted to know whether to add validation in Client in my work in progress. I will not and just allow it to get the same repeated URL for each owner.
`{owner}` | string | no | The owner ID to get details for | ||
|
||
The casing requirements of {owner} should be defined by the package source. | ||
The client should retain the casing of the username in whatever form it was found in other API responses or in the package repository signature. |
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.
👍 Sure, no manipulation of casing on Client
|
||
NuGet.org treats owner usernames in a case insensitive manner but not all package source may have this flexibility. | ||
|
||
For example, nuget.org's package details template looks like this: |
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 is talking about package details template, but the example is for an owner details template.
https://www.nuget.org/profile/{owner} | ||
``` | ||
|
||
If the client implementation needs to display a link to the package details for NuGet.Versioning 4.3.0, it would produce the following URL and provide it to the user: |
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 example appears to be from the package details template.
|
||
# Owner URL template | ||
|
||
It is possible for a client to build a URL that can be used by the user to see owner's page in their web browser. |
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.
It is possible for a client to build a URL that can be used by the user to see owner's page in their web browser. | |
It is possible for a client to build a URL that can be used by the user to see a package owner's profile page in their web browser. |
# Owner URL template | ||
|
||
It is possible for a client to build a URL that can be used by the user to see owner's page in their web browser. | ||
This is useful when a package source wants to show additional information about an owner that may not fit within the scope of what the NuGet client application shows. |
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'm not sure... maybe scratch this for now? As far as I know, nuget.org's owner profile pages are just a profile photo and listings of packages owned by that individual. They don't really add details... or can they?
From my spec's perspective, the purpose is to get some reassurance that the owner is really the owner I expect, by looking at their other published packages on this package source.
Learn Build status updates of commit e740e00: ✅ Validation status: passed
For more details, please refer to the build report. For any questions, please:
|
Design Spec: https://github.com/NuGet/Home/blob/dev/accepted/2023/owner-author-pmui.md#functional-explanation
Service Index PR: https://github.com/NuGet/NuGet.Services.Index/pull/235