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
RFC: Documentation-related Metadata #26
base: master
Are you sure you want to change the base?
RFC: Documentation-related Metadata #26
Changes from 8 commits
400afb3
facaf36
5249c25
f347bd5
cbd40e8
3f5db7e
4b70dc7
517d530
ab53e66
30e3ca8
bd7db72
3d3db60
6e299d7
b087168
78ef119
300b4cf
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.
What is the default value if any? What happens if no category is specified, is it considered as unclassified?
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 would be up to the consumer to decide which text to use to describe a lack of categorization. None of these fields would be considered required from the compiler standpoint.
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.
What is the default value?
Is there a way to a module is portable and available in all the experience out of the box? Listing all the experiences is not an option here, since we might add new experiences in the futures and modules hardcoding all the experiences will need to be updated to add the new experience to the list.
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.
yes that's a good point. An 'All' experience is reasonable. The risk is that people will tend to use that without testing their components in the experience, or new experiences may be added that some existing components don't support.
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.
In order to implement component grouping, we will need more information than just a boolean.
Have you thought of having a property called
validParentComponent
that would reference in which context this component can 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.
Yes that's a good idea.
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 am not aware of a standard way to mark a component as deprecated.
It makes sense to keep the
@deprecated
notation in JSDoc. Even if a module/component is not deprecated, some of their methods and properties might be.It is not uncommon that a deprecated module doesn't have a 1-1 replacement. Deprecated requires most of the time some free form text to indicate how to migrate, we can ask for deprecated modules to add a banner in the documentation explaining the migration strategy. We can also add another property called
deprecationMessage
to have a standard way to surface in the component library and the different tools.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 like the idea of
deprecationMessage
, as that can be used easier in tools like the editor LSPs. For more complicated scenarios I agree this could be supplemented with more information in the documentation itself.When I talked with Alex and Ravi it came up that a deprecation flag might be added to the
.js-meta.xml
file. I'm not sure if it belongs there, but I agree the@deprecated
tag works better than in the doc metadata, so how we flag deprecation can be a separate discussion.