Skip to content

jQuery.uniqueSort: Fixed category name#913

Closed
AurelioDeRosa wants to merge 2 commits into
jquery:masterfrom
AurelioDeRosa:category-fix
Closed

jQuery.uniqueSort: Fixed category name#913
AurelioDeRosa wants to merge 2 commits into
jquery:masterfrom
AurelioDeRosa:category-fix

Conversation

@AurelioDeRosa

Copy link
Copy Markdown
Member

Fixes gh-908

@dmethvin

Copy link
Copy Markdown
Member

Ah, so the encoded version works? Awesome!

@AurelioDeRosa

Copy link
Copy Markdown
Member Author

@agcolom did you have the chance to double-check this?

@agcolom

agcolom commented May 31, 2016

Copy link
Copy Markdown
Member

@AurelioDeRosa My local setup is no longer working for the core api (not sure what change broke it yet, and I cannot install anything as I get the error on npm install!) so I haven't been able to test this, sorry.

@kswedberg

Copy link
Copy Markdown
Member

@AurelioDeRosa this needs a corresponding <category> entry in the categories.xml file. Unfortunately, when I try to add <category name="Version 1.12&amp;2.2" slug="1.12&amp;2.2">…</category>, I get "Warning: Invalid slug: 1.12&2.2." Using a hyphen (-) works. Maybe we should use that instead?

@kswedberg

kswedberg commented Jun 16, 2016

Copy link
Copy Markdown
Member

@AurelioDeRosa ugh, I think I spoke too soon. While I don't get the error when I use <category name="Version 1.12-2.2" slug="1.12-2.2"> (with hyphens). The build process seems to ignore it. Maybe there's a stricter check when adding the version than when validating it?

@AurelioDeRosa

Copy link
Copy Markdown
Member Author

@kswedberg I managed to have the dash working. I think what was wrong is that $.uniqueSort() doesn't not include the category at the bottom of the file.

If everyone is happy, I can update the PR and merge it.

@kswedberg

Copy link
Copy Markdown
Member

Aha! You're correct, @AurelioDeRosa . That was the missing piece of the puzzle. Thanks so much for looking into this. Please update and merge.

@AurelioDeRosa

Copy link
Copy Markdown
Member Author

PR updated. I double checked and everything works correctly. As soon as I get approval, I'll merge it.

@mgol

mgol commented Jul 17, 2016

Copy link
Copy Markdown
Member

LGTM

@AurelioDeRosa AurelioDeRosa deleted the category-fix branch July 17, 2016 17:00
kswedberg pushed a commit to kswedberg/api.jquery.com that referenced this pull request Jul 17, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

6 participants