Spec: http://patches.fedorapeople.org/jquery/js-jquery-migrate.spec SRPM: http://patches.fedorapeople.org/jquery/js-jquery-migrate-1.2.1-1.fc20.src.rpm FAS: patches Description: This plugin can be used to detect and restore APIs or features that have been deprecated in jQuery and removed as of version 1.9. The plugin can be included with versions of jQuery as old as 1.6.4 to identify potential upgrade issues via its JQMIGRATE console warnings. However, the plugin is only required for version 1.9.0 or higher to restore deprecated and removed functionality. Additionally, this package automatically maintains a copy of jQuery and jQuery Migrate combined into a single file to permit an easier transition to newer versions of jQuery.
Fixed based on feedback from the js-sizzle review. Spec: http://patches.fedorapeople.org/jquery/js-jquery-migrate.spec SRPM: http://patches.fedorapeople.org/jquery/js-jquery-migrate-1.2.1-2.fc20.src.rpm * Tue Jun 03 2014 T.C. Hollingsworth <tchollingsworth> - 1.2.1-2 - follow the github SourceURL guidelines
Spec: https://patches.fedorapeople.org/jquery/js-jquery-migrate.spec SRPM: https://patches.fedorapeople.org/jquery/js-jquery-migrate-1.2.1-3.fc20.src.rpm * Tue Oct 21 2014 T.C. Hollingsworth <tchollingsworth> - 1.2.1-3 - typo and whitespace fixes
A quick fedora-review run reveals one issue: Issues: ======= - Package does not contain duplicates in %files. Note: warning: File listed twice: /usr/share/javascript/jquery- migrate/jquery+migrate.js See: http://fedoraproject.org/wiki/Packaging/Guidelines#DuplicateFiles I think you need to specify %dir before: %{_jsdir}/jquery-migrate and list the files inside one by one: %{_jsdir}/jquery-migrate/jquery-migrate.js %{_jsdir}/jquery-migrate/jquery-migrate.min.js Licence seems ok (MIT), but it's not specified in any of the source files (not a blocker). mock build.log shows some warnings during %build: Registering "grunt-git-authors" local Npm module tasks. >> Local Npm module "grunt-git-authors" not found. Is it installed? and similar for grunt-contrib-watch, grunt-contrib-jshint and grunt-contrib-qunit. Are these anything to worry about? I will try to do a more complete review during the Christmas break.
(In reply to Dominik 'Rathann' Mierzejewski from comment #3) > A quick fedora-review run reveals one issue: > > Issues: > ======= > - Package does not contain duplicates in %files. Fixed. I guess the %ghost files caused this. I also fixed a wrong-end-of-line-encoding rpmlint warning I hadn't noticed before. > Licence seems ok (MIT), but it's not specified in any of the source files > (not a blocker). Yeah that's unfortunately pretty common. :-( > mock build.log shows some warnings during %build: > Registering "grunt-git-authors" local Npm module tasks. > >> Local Npm module "grunt-git-authors" not found. Is it installed? > > and similar for grunt-contrib-watch, grunt-contrib-jshint and > grunt-contrib-qunit. > > Are these anything to worry about? No. grunt-git-authors is only needed for releasing (changelog generation). grunt-contrib-watch is for developers only: if you execute `grunt watch` it places inotify watches on the various projects and rebuilds everything if one changes. grunt-contrib-qunit is the only thing we _should_ have, as it's needed for running the tests. But, as the "q" in the name hints, it runs tests in a QtWebKit frame and thus won't work well in koji. :-( Note that grunt only displays these warnings because I request verbose output via `-v` in the build. I guess I could turn that off to avoid confusing messages, but I do find it convenient to have the extra output always there on the offchance it is needed when a problem arises in some future update. > I will try to do a more complete review during the Christmas break. Thanks for the review so far! I hope you and your family had a wonderful holiday. -- Spec: https://patches.fedorapeople.org/jquery/js-jquery-migrate.spec SRPM: https://patches.fedorapeople.org/jquery/js-jquery-migrate-1.2.1-4.fc20.src.rpm * Mon Dec 29 2014 T.C. Hollingsworth <tchollingsworth> - 1.2.1-4 - correct duplicate in files list - use correct end-of-line encoding in README.md file
Looks like you forgot to build js-jquery1 (bug 1078371), so this package is not yet installable. I rebuilt your js-jquery1 in a local mock and it seems fine. Two more questions: 1. Why are you rebuilding the provided scripts upon every new jquery1 install instead of building them once at build time and adding strict version dependencies? 2. What are the missing dependencies required for %check? [...] # missing dependencies #%%check #grunt [...] Formally taking review.
Ping?
Ping2?
(In reply to Dominik 'Rathann' Mierzejewski from comment #5) > Looks like you forgot to build js-jquery1 (bug 1078371), so this package is > not yet installable. I rebuilt your js-jquery1 in a local mock and it seems > fine. Sorry, I'll build it ASAP. (Would have built it already, but dist-git seems to be down at the moment, even though I just used it an hour ago to update some existing packages...) > Two more questions: > > 1. Why are you rebuilding the provided scripts upon every new jquery1 > install instead of building them once at build time and adding strict > version dependencies? That would require rebuilding js-jquery-migrate on every js-jquery1 update. That's easy enough for me, but a provenpackager who updates js-jquery1 might not be aware of this strict dependency, until it breaks updates-testing that is. It also makes things more difficult for power users that might want to apply a patch to js-jquery1 or update it locally. It also makes downgrades more difficult for system administrators. By doing it this way, the concatenated copy always uses whatever version of js-jquery1 the system administrator wants to install with no hassle. If this required an actual build process with nodejs as the main jquery-migrate.js file does, I wouldn't have done it this way, as the extra dependencies would be unacceptable and make the package unavailable on arches that nodejs does not run on. But since we're just concatenating two text files with standard shell tools that are already installed, it seems nicer to defer it to install-time to allow end users and packagers both more flexibility. > 2. What are the missing dependencies required for %check? > > [...] > # missing dependencies > #%%check > #grunt > [...] npm(grunt-contrib-qunit) and its dependencies. This is now documented in the spec file. > Formally taking review. Thanks! -- Spec: https://patches.fedorapeople.org/jquery/js-jquery-migrate.spec SRPM: https://patches.fedorapeople.org/jquery/js-jquery-2.1.1-4.fc20.src.rpm * Thu Feb 19 2015 T.C. Hollingsworth <tchollingsworth> - 1.2.1-5 - document the postinstall scriptlets - indicate the missing dependencies for tests
Sorry it took so long. Here are some issues found upon review. 1. %license is not used for marking license text file 2. qunit is bundled 3. (SHOULD item) tests are not being run Fixing 1 and 2 would be enough for this to be APPROVED, but it's be much appreciated if you took a stab at enabling the testsuite as well. Full details below: MUST: rpmlint must be run on the source rpm and all binary rpms the build produces. The output should be posted in the review. rpmlint is clean built packages: [rathann@sakura ~/build/mock/fedora-rawhide-x86_64/result]$ rpmlint js-jquery-migrate-1.2.1-5.fc23.*.rpm 2 packages and 0 specfiles checked; 0 errors, 0 warnings. installed package: Spawning container 0ea55ffe3e794d5d9661f659a90d59e5 on /var/lib/mock/fedora-rawhide-x86_64/root. Press ^] three times within 1s to kill container. sh-4.3# rpmlint js-jquery-migrate 1 packages and 0 specfiles checked; 0 errors, 0 warnings. MUST: The package must meet the Packaging Guidelines . While I don't like triggers and having files generated at install time, the actual method of using this library recommended by upstream is including both jquery.js and jquery-migrate.js, so what you're doing is simply a convenience for package consumers, so I guess this outweighs the issue of not being able to easily verify the jquery+migrate{,.min}.js contents using rpm -V. MUST: The package must be licensed with a Fedora approved license and meet the Licensing Guidelines . The source files don't have any license information, but the included LICENSE-MIT is sufficient. -> Please ask upstream to include license information in the source files as well. MUST: If (and only if) the source package includes the text of the license(s) in its own file, then that file, containing the text of the license(s) for the package must be included in %license. -> Please mark the LICENSE-MIT file with %license macro. MUST: The sources used to build the package must match the upstream source, as provided in the spec URL. Reviewers should use sha256sum for this task as it is used by the sources file once imported into git. If no upstream URL can be specified for this package, please see the Source URL Guidelines for how to deal with this. $ spectool -g js-jquery-migrate.spec Getting https://github.com/jquery/jquery-migrate/archive/54ee1ae5928423a0e44ba1861112a746181fbc35/js-jquery-migrate-54ee1ae5928423a0e44ba1861112a746181fbc35.tar.gz to ./js-jquery-migrate-54ee1ae5928423a0e44ba1861112a746181fbc35.tar.gz % Total % Received % Xferd Average Speed Time Time Time Current Dload Upload Total Spent Left Speed 100 163 0 163 0 0 36 0 --:--:-- 0:00:04 --:--:-- 38 100 52329 100 52329 0 0 7064 0 0:00:07 0:00:07 --:--:-- 24407 [rathann@sakura ~/build/SOURCES/js-jquery-migrate]$ sha256sum js-jquery-migrate-54ee1ae5928423a0e44ba1861112a746181fbc35.tar.gz js-jquery-migrate-54ee1ae5928423a0e44ba1861112a746181fbc35.tar.gz.orig 1b78a63145f11f2e946fb9dc36aa9b2a0d3e40aba558b89e24122f30b92e8625 js-jquery-migrate-54ee1ae5928423a0e44ba1861112a746181fbc35.tar.gz 1b78a63145f11f2e946fb9dc36aa9b2a0d3e40aba558b89e24122f30b92e8625 js-jquery-migrate-54ee1ae5928423a0e44ba1861112a746181fbc35.tar.gz.orig MUST: Packages must NOT bundle copies of system libraries. It's bundling qunit (https://qunitjs.com), even if it isn't used unless enable_tests is set to 1. -> Please unbundle. SHOULD: The reviewer should test that the package builds in mock. Builds fine in fedora-rawhide-x86_64 mock. SHOULD: The reviewer should test that the package functions as described. A package should not segfault instead of running, for example. You should try to run the testsuite at build time. You have the following comment in your spec: # disabled due to missing dependencies (that likely won't run in koji anyway) What is missing apart from qunit?
(In reply to Dominik 'Rathann' Mierzejewski from comment #9) > While I don't like triggers and having files generated at install time, the > actual method of using > this library recommended by upstream is including both jquery.js and > jquery-migrate.js, so what > you're doing is simply a convenience for package consumers, so I guess this > outweighs the issue > of not being able to easily verify the jquery+migrate{,.min}.js contents > using rpm -V. I added a %verifyscript to address this concern. Now if these generated files are not consistent with the already-verified RPM-installed files, rpm -V will complain: ---------- % rpm -Vv js-jquery-migrate ......... /usr/share/doc/js-jquery-migrate ......... d /usr/share/doc/js-jquery-migrate/AUTHORS.txt ......... d /usr/share/doc/js-jquery-migrate/CONTRIBUTING.md ......... d /usr/share/doc/js-jquery-migrate/LICENSE-MIT ......... d /usr/share/doc/js-jquery-migrate/README.md ......... /usr/share/javascript/jquery-migrate ......... /usr/share/javascript/jquery-migrate/jquery-migrate.js ......... /usr/share/javascript/jquery-migrate/jquery-migrate.min.js % sudo truncate --size=10 /usr/share/javascript/jquery-migrate/jquery+migrate.js % rpm -Vv js-jquery-migrate ......... /usr/share/doc/js-jquery-migrate ......... d /usr/share/doc/js-jquery-migrate/AUTHORS.txt ......... d /usr/share/doc/js-jquery-migrate/CONTRIBUTING.md ......... d /usr/share/doc/js-jquery-migrate/LICENSE-MIT ......... d /usr/share/doc/js-jquery-migrate/README.md ......... /usr/share/javascript/jquery-migrate ......... /usr/share/javascript/jquery-migrate/jquery-migrate.js ......... /usr/share/javascript/jquery-migrate/jquery-migrate.min.js The file /usr/share/javascript/jquery+migrate.js does not match the installed versions of js-jquery1 and js-jquery-migrate. Please reinstall these packages to resolve this issue. ----------- > MUST: If (and only if) the source package includes the text of the > license(s) in its own file, then that file, containing the text of the > license(s) for the package must be included in %license. > > -> Please mark the LICENSE-MIT file with %license macro. Fixed. > MUST: Packages must NOT bundle copies of system libraries. > > It's bundling qunit (https://qunitjs.com), even if it isn't used unless > enable_tests is set to 1. > -> Please unbundle. Fixed. > SHOULD: The reviewer should test that the package functions as described. A > package should not segfault instead of running, for example. > > You should try to run the testsuite at build time. You have the following > comment in your spec: > # disabled due to missing dependencies (that likely won't run in koji anyway) > What is missing apart from qunit? Its dependencies. (Including what will be the fourth nodejs command line argument parsing library we've packaged. Yeesh!) -- Spec: https://patches.fedorapeople.org/jquery/js-jquery-migrate.spec SRPM: https://patches.fedorapeople.org/jquery/js-jquery-migrate-1.2.1-6.fc20.src.rpm * Sat May 16 2015 T.C. Hollingsworth <tchollingsworth> - 1.2.1-6 - use %%license where available - remove bundled qunit - add %%verifyscript for files generated post-install
Excellent! This package is APPROVED.
New Package SCM Request ======================= Package Name: js-jquery-migrate Short Description: APIs and features removed from jQuery core Upstream URL: https://github.com/jquery/jquery-migrate/ Owners: patches jamielinux Branches: f22 f21 f20 epel7 el6 el5 InitialCC:
Git done (by process-git-requests).
T.C., it looks like this isn't built yet. Could you build the package and close this bug?
Sorry for the delay. http://koji.fedoraproject.org/koji/buildinfo?buildID=663845
js-jquery-migrate-1.2.1-6.el7 has been submitted as an update for Fedora EPEL 7. https://admin.fedoraproject.org/updates/js-jquery-migrate-1.2.1-6.el7
js-jquery-migrate-1.2.1-6.el6 has been submitted as an update for Fedora EPEL 6. https://admin.fedoraproject.org/updates/js-jquery-migrate-1.2.1-6.el6
js-jquery-migrate-1.2.1-6.fc22 has been submitted as an update for Fedora 22. https://admin.fedoraproject.org/updates/js-jquery-migrate-1.2.1-6.fc22
js-jquery-migrate-1.2.1-6.fc21 has been submitted as an update for Fedora 21. https://admin.fedoraproject.org/updates/js-jquery-migrate-1.2.1-6.fc21