Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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?
Add API docs for owners' template resource. #3282
Changes from 1 commit
b8de56d
7d33f49
d2d7f17
786f6e2
e740e00
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
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.
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.
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.