-
Notifications
You must be signed in to change notification settings - Fork 1.6k
Increase number of Tags allowed for 1.12 manifests #5784
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
base: master
Are you sure you want to change the base?
Conversation
dkbennett
left a comment
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.
A few points here:
-
It is very late in the release for a schema change, so this should go into 1.13 after 1.12 is released.
-
Unsure what sort of impact on the UI or index if someone were to actually have 64 tags.
-
We would want a test case for that and to update the manifest schema sample to include an entry with preferably 64 tags of respectable size (perhaps find some long words to use) to verify the behavior actually works and that tag size doesn't cause any issues downstream or when enumerated.
-
A final point - should manifests have that many tags? A smaller size means people might be more concise and judicious with their tag usage, while a large number would allow tag spam to increase discoverability and might be a net negative to the product.
Putting a "request changes" specifically for the tests on the manifests. If we just wait the 'latest' will be come 1.13 (I'm doing this for some other changes with uninstall switches). Also for folks like @denelon and @JohnMcPMS to weigh in on whether tag count increase is a good thing.
|
@DandelionSprout do you have a "good" example where more than 16 makes sense and isn't just spamming for more hits? |
|
As far as I can see, the UI and index creation tools will handle an increase seamlessly - they are already abstracted to simply iterate through the collection of tags. The only item I see would be a potential increase in index size as more tags are added. Additionally, I don’t see how any additional tests beyond what is currently in the codebase would provide benefit. The current tests make sure that the tags field (at least up to a few tags) gets populated and retrieved correctly. In my opinion this is actually an extremely minor change, with almost no technical risk. The length of a single tag is something that, if it were an issue, would be an issue in the existing code anyways; and the only places tags are consumed are 1) building and searching sources, and 2) Showing a manifest. The Perhaps I could add a manifest into TestData with a large number of tags to be certain the test index will build successfully, but I don't see what other tests would cover areas that aren’t already tested |
|
|
And... I just thought about PowerToys which has nearly 30 utilities. |
|
I'm going to mark this as a draft until the manifest schema version is bumped |
Microsoft Reviewers: Open in CodeFlow