Details
Improvement
Trivial
Description
As discussed with steffkes on IRC
Issue Links
- relates to
-
SOLR-7588
naturalSort.js is provided as coffeescript instead of plain javascript
-
- Closed
-
Activity
- All
- Comments
- Work Log
- History
- Activity
- Transitions
That -2 patch looks really good.
It looks like you re-used somebody else's code, specifically this project:
https://github.com/overset/javascript-natural-sort/blob/master/naturalSort.js
This isn't a problem. I believe we should probably use their source file which includes the author and the license, which is MIT, and compatible with Solr's license. I wonder if we need to include a license and notice file in solr/licenses for this code.
Regarding patch attachments, normally they really should be just named after the issue, for every upload, which means they all get named SOLR-7409.patch. If I am making an alternate proposal on an issue that somebody else is working, I will sometimes do a name like SOLR-7409-elyograg.patch to keep it separate from the more official work in progress. Jira will automatically grey out all but the newest version of each attachment that has the same name, but the older ones will remain available. This is in line with the open source ideal of keeping all discussion and development in the public view.
New version of patch with CHANGES.txt, license, notice, and the source file from the naturalsort project with author and license.
Jellyfrog, does the credit in CHANGES.txt look OK to you, or would you like it to say something else?
Fellow committers, does all this look OK? I'm particularly concerned to make sure I'm handling the licensing issues properly, to keep the project out of legal trouble.
Fine for me!
Its acutely this fork; https://github.com/jarinudom/naturalSort.js btw.
I didn't look at the differences between those two javascript files, but I can see that there ARE differences, so I grabbed the version of naturalSort.js found in the fork you linked. New patch with those changes. This resulted in a minor change to the license file as well – this version has an actual copyright year, before I guessed 2012 because that's when the last commit to the repo was.
Slight patch modification. I grabbed the actual LICENSE file from the github fork and put it in as the license file in the patch. The previous version was copied from another MIT license file already included in solr/licenses.
On precommit, it seems that rat requires a license header in the .js source file. I have added one with the MIT license. Once precommit passes, I will upload a new patch.
Tests and precommit pass with this patch.
Commit 1677365 from Shawn Heisey in branch 'dev/trunk'
[ https://svn.apache.org/r1677365 ]
SOLR-7409: Sort datimport handlers in the admin UI
Commit 1677366 from Shawn Heisey in branch 'dev/branches/branch_5x'
[ https://svn.apache.org/r1677366 ]
SOLR-7409: Sort datimport handlers in the admin UI (backport trunk r1677365)
Commit 1684234 from Shawn Heisey in branch 'dev/trunk'
[ https://svn.apache.org/r1684234 ]
SOLR-7588: Fix javascript bug introduced by SOLR-7409 that broke dataimport in admin UI.
Commit 1684235 from Shawn Heisey in branch 'dev/branches/branch_5x'
[ https://svn.apache.org/r1684235 ]
SOLR-7588: Fix javascript bug introduced by SOLR-7409 that broke dataimport in admin UI. (merge trunk r1684234)
Commit 1684236 from Shawn Heisey in branch 'dev/branches/lucene_solr_5_2'
[ https://svn.apache.org/r1684236 ]
SOLR-7588: Fix javascript bug introduced by SOLR-7409 that broke dataimport in admin UI. (merge trunk r1684234)
Bulk close for 5.2.0.
Pick one of the patches only!
Without any of the patches
With
SOLR-7409-1.patchWith
SOLR-7409-2.patch