https://bugzilla.redhat.com/show_bug.cgi?id=2091282 --- Comment #4 from Ben Beasley <code@xxxxxxxxxxxxxxxxxx> --- Thank you for the preliminary review! > Comments: > 1) Seems good to me. > > 2) The url http://people.mozilla.org/~jkew/woff/ seems not to be available. > Code seems to be available at > https://web.archive.org/web/20170630235618/https://people-mozilla.org/~jkew/ > woff/ as indicated at https://github.com/TheJessieKirk/sfnt2woff . That’s true. That URL is from the following comment, excerpted from the upstream license file: > # Sources in the top-level directory belong to, or are based on, the WOFF > # reference implementation originally published at > # http://people.mozilla.org/~jkew/woff/, and are distributed under the terms > # specified in the file LICENSE-WOFF (MPL 1.1/GPL 2.0/LGPL 2.1). The URL is still a useful identifier for which software was forked and extended to create this program even though the original upstream for the reference implementation (packaged in https://src.fedoraproject.org/rpms/woff) has disappeared. I’m not opposed to adding the archive.org and GitHub mirror links to a new spec file comment for informational purposes, although I don’t believe there is any requirement to have a working URL for software that I am mentioning but not packaging. > 3) Descriptions at > https://packages.fedoraproject.org/pkgs/woff2/woff2-tools/ and > https://launchpad.net/ubuntu/bionic/+package/woff-tools , indicate both > compression and decompression executables are available, maybe the > description could be updated to indicate this as the name of the package > suggests only compression is possible, though decompression only seems to be > to stdout. That’s reasonable. The description is taken from this software’s upstream, but I could add a sentence mentioning that there is a matching decompression utility. > 4) Fedora does not recommend packaging WOFF fonts > https://docs.fedoraproject.org/en-US/packaging-guidelines/FontsPolicy/ > #_web_fonts > > 5) WOFF2 https://www.w3.org/TR/WOFF2/ is available and may compress better. > Some of the implementations listed at > https://www.w3.org/Fonts/WG/WOFF2/Implementation.html have been packaged. Note that no fonts are packaged here. Your comments are all correct, but I don’t believe they affect this package. This tool is still useful for end-users who have reasons to work with the obsolete WOFF format, and as a dependency for other font creation and development tools with more general applicability. For example, it is a dependency for https://github.com/theleagueof/fontship, which is used for compiling certain open-source fonts from their design sources. It would be really nice to package *that* in Fedora. > 6) Should header files be in a devel package? See https://docs.fedoraproject.org/en-US/packaging-guidelines/#_devel_packages Headers that are installed (i.e., because they are needed for linking against a library offered by the package, or they constitute a header-only library provided by the package) should be in a -devel package. There is no reason to attempt to install non-API headers that are internal sources—only those that are “public” headers used for developing with the packaged software. Since this package only provides a pair of command-line tools without any sort of accompanying API or library, it’s correct that no header files are installed. -- You are receiving this mail because: You are always notified about changes to this product and component You are on the CC list for the bug. https://bugzilla.redhat.com/show_bug.cgi?id=2091282 _______________________________________________ package-review mailing list -- package-review@xxxxxxxxxxxxxxxxxxxxxxx To unsubscribe send an email to package-review-leave@xxxxxxxxxxxxxxxxxxxxxxx Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedoraproject.org/archives/list/package-review@xxxxxxxxxxxxxxxxxxxxxxx Do not reply to spam on the list, report it: https://pagure.io/fedora-infrastructure
