[Bug 1103414] Review Request: js-jquery-migrate - APIs and features removed from jQuery core

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



https://bugzilla.redhat.com/show_bug.cgi?id=1103414



--- Comment #9 from Dominik 'Rathann' Mierzejewski <dominik@xxxxxxxxxxxxxx> ---
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?

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
_______________________________________________
package-review mailing list
package-review@xxxxxxxxxxxxxxxxxxxxxxx
https://admin.fedoraproject.org/mailman/listinfo/package-review





[Index of Archives]     [Fedora Legacy]     [Fedora Desktop]     [Fedora SELinux]     [Yosemite News]     [KDE Users]     [Fedora Tools]