-
Notifications
You must be signed in to change notification settings - Fork 180
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
initial text for Label changes #1001
Conversation
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.
woops I made these comments the other day but forgot to submit the review. Anyway this looks pretty sensible to me -- I'm open to being convinced about both comments.
##### Asset names | ||
|
||
The Label Extension recommends assets be named with the keys "raster_labels" or "vector_labels". However, this is just a recommendation, and acutal name are entirely at the discretion of the implementer. | ||
|
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.
##### Asset names | |
The Label Extension recommends assets be named with the keys "raster_labels" or "vector_labels". However, this is just a recommendation, and acutal name are entirely at the discretion of the implementer. |
kind of the inverse of the previous comment -- I think tooling developers would be better off if there weren't recommended names for things, since the lines between recommended, required, and "required for this one tool to work" aren't always clearly drawn.
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 agree. I only left this in the spec b/c it was the required way to indicate labels previously. I'm fine with removing it, but I'm worried that their might be tools that rely on it? But, maybe better just to remove it now before 1.0.0?
Related Issue(s): #936
Proposed Changes:
PR Checklist: