[Bug 1024589] Review Request: zlib-js - JavaScript library reimplementing compression
https://bugzilla.redhat.com/show_bug.cgi?id=1024589 Tom Hughes t...@compton.nu changed: What|Removed |Added Flags|fedora-review? |fedora-review+ --- Comment #13 from Tom Hughes t...@compton.nu --- Looks good now then. Package approved. -- 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@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 1024589] Review Request: zlib-js - JavaScript library reimplementing compression
https://bugzilla.redhat.com/show_bug.cgi?id=1024589 --- Comment #5 from Zbigniew Jędrzejewski-Szmek zbys...@in.waw.pl --- Tom, sorry for the slow response. I think that the naming proposed by Jamie is OK: nodejs-zlibjs and js-zlib. The tarball is called zlibjs, so even upstream seems to skip the dot. Jamie, thank you for the updated spec file. It seems much better than my version. I made two changes though: - I reverted your replacement of sed with patch, since the patch seems very fragile and will probably have to be updated quite often when upstream changes the ant file - I simplified the arch condition using grep May I add you as a co-maintainer? Spec URL: http://in.waw.pl/~zbyszek/fedora/zlib-js.spec SRPM URL: http://in.waw.pl/~zbyszek/fedora/zlib-js-0.2.0-3.fc20.src.rpm -- 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@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 1024589] Review Request: zlib-js - JavaScript library reimplementing compression
https://bugzilla.redhat.com/show_bug.cgi?id=1024589 --- Comment #6 from Zbigniew Jędrzejewski-Szmek zbys...@in.waw.pl --- Argh, wrong url: Spec URL: http://in.waw.pl/~zbyszek/fedora/js-zlib.spec SRPM URL: http://in.waw.pl/~zbyszek/fedora/js-zlib-0.2.0-3.fc20.src.rpm -- 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@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 1024589] Review Request: zlib-js - JavaScript library reimplementing compression
https://bugzilla.redhat.com/show_bug.cgi?id=1024589
Jamie Nguyen jamieli...@fedoraproject.org changed:
What|Removed |Added
Blocks||1077322
--- Comment #7 from Jamie Nguyen jamieli...@fedoraproject.org ---
To paraphrase a discussion with T.C. via email:
What I thought was a working solution turns out to be not really working at
all. %{_arch} is the build host architecture not the target architecture, so if
an EPEL package is sent to a x86_64 builder then the nodejs-zlibjs subpackage
will be unconditionally present even in PPC repositories. And conversely, if
the package is sent to a PPC builder then the nodejs-zlibjs subpackage will be
unconditionally absent even in x86_64 repositories. The only solution with our
current buildsystem appears to be to make the package arched (remove BuildArch:
noarch, and use %ifarch for the subpackage).
I suggested instead that we split them into two completely separate packages
and symlink js-zlib/node-zlib.js to the nodejs-zlibjs package. This would
allow:
- js-zlib to remain completely free of architecture restrictions
- nodejs-zlibjs to be restricted to %{nodejs_arches}
- both packages to remain noarch
I have confirmed by running the test suite that the new nodejs-zlibjs package
works as intended with the symlink.
Review Request: nodejs-zlibjs
https://bugzilla.redhat.com/show_bug.cgi?id=1077322
Spec URL: http://jamielinux.fedorapeople.org/gruntjs/js-zlib.spec
SRPM URL:
http://jamielinux.fedorapeople.org/gruntjs/SRPMS/js-zlib-0.2.0-4.fc21.src.rpm
* Mon Mar 17 2014 Jamie Nguyen jamieli...@fedoraproject.org - 0.2.0-4
- As it turns out, matching {_arch} won't solve our problem as it indicates
the architecture of the build host not the target architecture. Instead
split nodejs-zlibjs into a separate package, as otherwise js-zlib would be
restricted to {nodejs_arches}. bin/node-zlib.js will remain in the js-zlib
package, while nodejs-zlibjs will depend on js-zlib and have a symlink.
- add logic for building on EPEL 6
Referenced Bugs:
https://bugzilla.redhat.com/show_bug.cgi?id=1077322
[Bug 1077322] Review Request: nodejs-zlibjs - JavaScript library
reimplementing compression, made available for Node.js
--
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@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 1024589] Review Request: zlib-js - JavaScript library reimplementing compression
https://bugzilla.redhat.com/show_bug.cgi?id=1024589 Jamie Nguyen jamieli...@fedoraproject.org changed: What|Removed |Added Blocks|977137 | Referenced Bugs: https://bugzilla.redhat.com/show_bug.cgi?id=977137 [Bug 977137] Review Request: nodejs-zlib-browserify - Wrapper for zlib.js to allow for use in browsers -- 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@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 1024589] Review Request: zlib-js - JavaScript library reimplementing compression
https://bugzilla.redhat.com/show_bug.cgi?id=1024589 --- Comment #8 from Jamie Nguyen jamieli...@fedoraproject.org --- (And in case it wasn't obvious, the symlink is so that we aren't building the same source twice, which would seem less than ideal.) -- 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@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 1024589] Review Request: zlib-js - JavaScript library reimplementing compression
https://bugzilla.redhat.com/show_bug.cgi?id=1024589
--- Comment #9 from Tom Hughes t...@compton.nu ---
Package Review
==
Legend:
[x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated
[ ] = Manual review needed
Issues
==
[!]: Rpmlint is run on all installed packages.
js-zlib.noarch: W: spelling-error Summary(en_US) reimplementing - re
implementing, re-implementing, implementing
js-zlib.src: W: spelling-error Summary(en_US) reimplementing - re
implementing, re-implementing, implementing
= MUST items =
Generic:
[x]: Package is licensed with an open-source compatible license and meets
other legal requirements as defined in the legal section of Packaging
Guidelines.
[x]: License field in the package spec file matches the actual license.
Note: Checking patched sources after %prep for licenses. Licenses found:
Apache (v2.0), Unknown or generated. 4 files have unknown license.
Detailed output of licensecheck in /home/tom/1024589-js-
zlib/licensecheck.txt
[x]: Package contains no bundled libraries without FPC exception.
[x]: Changelog in prescribed format.
[x]: Sources contain only permissible code or content.
[-]: Package contains desktop file if it is a GUI application.
[-]: Development files must be in a -devel package
[x]: Package uses nothing in %doc for runtime.
[x]: Package consistently uses macros (instead of hard-coded directory names).
[ ]: Package is named according to the Package Naming Guidelines.
[x]: Package does not generate any conflict.
[x]: Package obeys FHS, except libexecdir and /usr/target.
[-]: If the package is a rename of another package, proper Obsoletes and
Provides are present.
[x]: Requires correct, justified where necessary.
[x]: Spec file is legible and written in American English.
[-]: Package contains systemd file(s) if in need.
[x]: Package is not known to require an ExcludeArch tag.
[-]: Large documentation must go in a -doc subpackage. Large could be size
(~1MB) or number of files.
Note: Documentation size is 30720 bytes in 4 files.
[x]: Package complies to the Packaging Guidelines
[x]: Package successfully compiles and builds into binary rpms on at least one
supported primary architecture.
[x]: Package installs properly.
[x]: Rpmlint is run on all rpms the build produces.
Note: There are rpmlint messages (see attachment).
[x]: 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 is included in %doc.
[x]: Package requires other packages for directories it uses.
[x]: Package must own all directories that it creates.
[x]: Package does not own files or directories owned by other packages.
[x]: All build dependencies are listed in BuildRequires, except for any that
are listed in the exceptions section of Packaging Guidelines.
[x]: Package uses either %{buildroot} or $RPM_BUILD_ROOT
[x]: Package does not run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) at the
beginning of %install.
[x]: Macros in Summary, %description expandable at SRPM build time.
[x]: Package does not contain duplicates in %files.
[x]: Permissions on files are set properly.
[x]: Package use %makeinstall only when make install' ' DESTDIR=... doesn't
work.
[x]: Package is named using only allowed ASCII characters.
[x]: Package do not use a name that already exist
[x]: Package is not relocatable.
[x]: Sources used to build the package match the upstream source, as provided
in the spec URL.
[x]: Spec file name must match the spec package %{name}, in the format
%{name}.spec.
[x]: File names are valid UTF-8.
[x]: Packages must not store files under /srv, /opt or /usr/local
= SHOULD items =
Generic:
[x]: Packager, Vendor, PreReq, Copyright tags should not be in spec file
[-]: If the source package does not include license text(s) as a separate file
from upstream, the packager SHOULD query upstream to include it.
[x]: Final provides and requires are sane (see attachments).
[?]: Package functions as described.
[x]: Latest version is packaged.
[x]: Package does not include license text files separate from upstream.
[-]: Description and summary sections in the package spec file contains
translations for supported Non-English languages, if available.
[x]: Package should compile and build into binary rpms on all supported
architectures.
[?]: %check is present and all tests pass.
[x]: Packages should try to preserve timestamps of original installed files.
[x]: Sources can be downloaded from URI in Source: tag
[x]: Reviewer should test that the package builds in mock.
[x]: Buildroot is not present
[x]: Package has no %clean section with rm -rf %{buildroot} (or
$RPM_BUILD_ROOT)
[x]: Dist tag is present (not strictly required in GL).
[x]: No file requires outside of /etc, /bin, /sbin, /usr/bin, /usr/sbin.
[x]: SourceX is a working URL.
[x]: Spec use %global instead of
[Bug 1024589] Review Request: zlib-js - JavaScript library reimplementing compression
https://bugzilla.redhat.com/show_bug.cgi?id=1024589 --- Comment #10 from Tom Hughes t...@compton.nu --- Other than the rpmlint warning, my other question is whether we should be packaging the .map files? -- 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@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 1024589] Review Request: zlib-js - JavaScript library reimplementing compression
https://bugzilla.redhat.com/show_bug.cgi?id=1024589 --- Comment #11 from Jamie Nguyen jamieli...@fedoraproject.org --- Hmmm. They would be useful to have. I'm not sure what the guidelines would say about having the *.map in jsdir along with the rest of the minified javascript, but since they are useful in the web browser then I think it makes sense to put them in jsdir. Spec URL: http://jamielinux.fedorapeople.org/gruntjs/js-zlib.spec SRPM URL: http://jamielinux.fedorapeople.org/gruntjs/SRPMS/js-zlib-0.2.0-5.fc21.src.rpm * Mon Mar 17 2014 Jamie Nguyen jamieli...@fedoraproject.org - 0.2.0-5 - also include .map files -- 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@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 1024589] Review Request: zlib-js - JavaScript library reimplementing compression
https://bugzilla.redhat.com/show_bug.cgi?id=1024589 --- Comment #12 from Zbigniew Jędrzejewski-Szmek zbys...@in.waw.pl --- Package looks OK. The rpmlint warning is bogus: reimplemented is what is commonly used, and re-implemented looks foreign. -- 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@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 1024589] Review Request: zlib-js - JavaScript library reimplementing compression
https://bugzilla.redhat.com/show_bug.cgi?id=1024589
--- Comment #4 from Jamie Nguyen jamieli...@fedoraproject.org ---
Spec URL: http://jamielinux.fedorapeople.org/gruntjs/js-zlib.spec
SRPM URL:
http://jamielinux.fedorapeople.org/gruntjs/SRPMS/js-zlib-0.2.0-2.fc21.src.rpm
* Fri Mar 14 2014 Jamie Nguyen jamieli...@fedoraproject.org - 0.2.0-2
- add missing BuildArch: noarch
- nodejs-zlibjs should only be built for {nodejs_arches} but js-zlib package
does not have this limitation, so add logic for building nodejs-zlibjs only
on {nodejs_arches} while allowing js-zlib to be built on all architectures
- add logic for building on EPEL 6
OK so if you think this looks like one big hack... well it is. But it works!
Until there is some other solution, this will do. Any arches that get added to
the %{nodejs_arches} macro will have to be manually added to this Spec.
From the Spec:
The following longwinded conditionals are required because both js-zlib and
nodejs-zlibjs are 'BuildArch: noarch', but the nodejs-zlibjs package is
additionally restricted to 'ExclusiveArch: {nodejs_arches} noarch' (while still
remaining a noarch package). We want to build js-zlib on all arches, but
unfortunately the 'ifarch' conditional cannot be combined with 'BuildArch:
noarch'. This rules out the use of 'ifarch {nodejs_arches}' and we instead have
to match {_arch} with each architecture we want to include.
--
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@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 1024589] Review Request: zlib-js - JavaScript library reimplementing compression
https://bugzilla.redhat.com/show_bug.cgi?id=1024589
Jamie Nguyen jamieli...@fedoraproject.org changed:
What|Removed |Added
CC||jamieli...@fedoraproject.or
||g
--- Comment #3 from Jamie Nguyen jamieli...@fedoraproject.org ---
I don't mean to barge in uninvited, but here is my take on the package. Feel
free to ignore my Spec. (There are also lots of whitespace changes, sorry.)
NB: This is unlikely to be available on anything other than F21 and above.
closure-compiler requires guava. F20 has too old a version of guava, and EL6 is
missing several BuildRequires for guava. Since this is part of the long and
winding jQuery dependency tree, we won't see jQuery in F20/F19/EL6 unless some
magic happens.
I decided to package from NPM as it contains everything we need. I also decided
to maintain the names and locations of files in the upstream software to avoid
end-user confusion.
Having recently introduced a package called js-json, containing the most
popular(?) json JS library, I thought the most appropriate name would be
js-zlib, containing the most popular(?) zlib library.
Spec URL: http://jamielinux.fedorapeople.org/gruntjs/js-zlib.spec
SRPM URL:
http://jamielinux.fedorapeople.org/gruntjs/SRPMS/js-zlib-0.2.0-1.fc21.src.rpm
* Thu Mar 13 2014 Jamie Nguyen jamieli...@fedoraproject.org - 0.2.0-1
- update to upstream release 0.2.0
- rename to js-zlib and nodejs-zlibjs
- install to {_jsdir}
- maintain filenames and locations that upstream are using
- use patch instead of sed
- include additional documentation in nodejs-zlibjs package
--
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@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 1024589] Review Request: zlib-js - JavaScript library reimplementing compression
https://bugzilla.redhat.com/show_bug.cgi?id=1024589 Tom Hughes t...@compton.nu changed: What|Removed |Added Status|NEW |ASSIGNED CC||t...@compton.nu Assignee|nob...@fedoraproject.org|t...@compton.nu Flags||fedora-review? -- 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@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 1024589] Review Request: zlib-js - JavaScript library reimplementing compression
https://bugzilla.redhat.com/show_bug.cgi?id=1024589
--- Comment #2 from Tom Hughes t...@compton.nu ---
First things first, latest upstream release is 0.2.0 so this needs updating to
that.
Next, and before I do a full review, I think we need to establish what rules we
are working to here. There are two packages which come out of this:
* The pure javascript one, which needs to comply with the javascript guidelines
(so should install in %{_jsdir} not %{_webassetdir}) and should be called
js-zlib-js I think. That is rather ugly I admit but it seems to be the logical
consequence of the naming rules...
* The node package, which needs to comply with the node guidelines, and should
be called nodejs-zlibjs (note the node module has no dot in the name per
https://www.npmjs.org/package/zlibjs).
What I'm not sure about is whether it is best to package this from git as you
have done, or from the NPM registry as the node guidelines would normally
require.
That also goes to the question of what the source rpm should be called...
--
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@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 1024589] Review Request: zlib-js - JavaScript library reimplementing compression
https://bugzilla.redhat.com/show_bug.cgi?id=1024589 Sandro Mani manisan...@gmail.com changed: What|Removed |Added Status|NEW |ASSIGNED CC||manisan...@gmail.com Assignee|nob...@fedoraproject.org|manisan...@gmail.com Flags||fedora-review? -- 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@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 1024589] Review Request: zlib-js - JavaScript library reimplementing compression
https://bugzilla.redhat.com/show_bug.cgi?id=1024589 Sandro Mani manisan...@gmail.com changed: What|Removed |Added Status|ASSIGNED|NEW Assignee|manisan...@gmail.com|nob...@fedoraproject.org Flags|fedora-review? | --- Comment #1 from Sandro Mani manisan...@gmail.com --- Uh sorry was a bit too quick, I'd rather leave node.js to someone else :S -- 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@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 1024589] Review Request: zlib-js - JavaScript library reimplementing compression
https://bugzilla.redhat.com/show_bug.cgi?id=1024589 Bug 1024589 depends on bug 1023848, which changed state. Bug 1023848 Summary: Review Request: closure-compiler - JavaScript minifier and checker https://bugzilla.redhat.com/show_bug.cgi?id=1023848 What|Removed |Added Status|ASSIGNED|CLOSED Resolution|--- |RAWHIDE -- 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@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 1024589] Review Request: zlib-js - JavaScript library reimplementing compression
https://bugzilla.redhat.com/show_bug.cgi?id=1024589 Zbigniew Jędrzejewski-Szmek zbys...@in.waw.pl changed: What|Removed |Added Blocks||977137 Referenced Bugs: https://bugzilla.redhat.com/show_bug.cgi?id=977137 [Bug 977137] Review Request: nodejs-zlib-browserify - Wrapper for zlib.js to allow for use in browsers -- You are receiving this mail because: You are on the CC list for the bug. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 1024589] Review Request: zlib-js - JavaScript library reimplementing compression
https://bugzilla.redhat.com/show_bug.cgi?id=1024589 Zbigniew Jędrzejewski-Szmek zbys...@in.waw.pl changed: What|Removed |Added Depends On||1023848 Referenced Bugs: https://bugzilla.redhat.com/show_bug.cgi?id=1023848 [Bug 1023848] Review Request: closure-compiler - JavaScript minifier and checker -- You are receiving this mail because: You are on the CC list for the bug. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
