SPEC URL: https://raw.githubusercontent.com/eclipseo/packaging/4549d76/aftertheflood-spark-fonts.spec SRPM URL: https://copr-be.cloud.fedoraproject.org/results/eclipseo/aftertheflood-spark/fedora-rawhide-x86_64/00603853-aftertheflood-spark-fonts/aftertheflood-spark-fonts-20170907-1.fc28.src.rpm Description: After the Flood Spark is a font that allows for the combination of text and visual data to show an idea and evidence in one headline. This builds on the principle of Sparklines defined by Edward Tufte and makes them easier to use. Sparklines are currently available as plugins or javascript elements. By installing the Spark font you can use them immediately without the need for custom code. Spark data needs to be formatted as comma-separated values, with curly brackets at both ends of the set, e.g., {30,60,90}. You can also have numbers at the beginning and end of the set, which are useful for providing the start and end points, e.g., 123{30,60,90}456 – Spark has numerals built in. Fedora account username: eclipseo Koji build: https://koji.fedoraproject.org/koji/taskinfo?taskID=21929480
This package seems okay aside from some font-specific things. I'm not totally qualified to review those, but from what I can see: Version is a bit weird. Upstream has no tags, but the webpage claims that "Version 1 was released". The spec uses a date but pulls from a commit; this doesn't follow the snapshot versioning guidelines. From https://fedoraproject.org/wiki/Packaging:FontsPolicy: > Fonts SHOULD be built from source whenever upstream provides them in a source format They are provided but I'm not sure how easy it is to build from them as there don't appear to be any instructions. Since you were able to work with upstream to get the license added, you can suggest to them to add the "license description" (#13) or "license info URL" (#14) fields in the fonts themselves. Speaking of licensing, MIT is not one of the recommended or explicitly approved licenses for fonts: https://fedoraproject.org/wiki/Legal_considerations_for_fonts#Approved_font_licenses That page is a bit old (it still points to the old legal list) but I could not find any MIT+font references in the archives, so I guess you'd have to clarify that one. Alternatively, upstream might be amenable to a more font-friendly license. Package Review ============== Legend: [x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated [ ] = Manual review needed ===== MUST items ===== Generic: [!]: Package is licensed with an open-source compatible license and meets other legal requirements as defined in the legal section of Packaging Guidelines. [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 %license. [x]: License field in the package spec file matches the actual license. Note: Checking patched sources after %prep for licenses. Licenses found: "MIT/X11 (BSD like)", "Unknown or generated". 1141 files have unknown license. Detailed output of licensecheck in review/1492475-aftertheflood-spark-fonts/licensecheck.txt [x]: Package contains no bundled libraries without FPC exception. [x]: Changelog in prescribed format. [x]: Sources contain only permissible code or content. [-]: Development files must be in a -devel package [-]: Package uses nothing in %doc for runtime. [x]: Package consistently uses macros (instead of hard-coded directory names). [x]: 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. [x]: Package is not known to require an ExcludeArch tag. [x]: Large documentation must go in a -doc subpackage. Large could be size (~1MB) or number of files. Note: Documentation size is 10240 bytes in 1 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]: 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]: Dist tag is present. [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 does not use a name that already exists. [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: [-]: 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. [?]: 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]: Reviewer should test that the package builds in mock. [x]: Buildroot is not present [x]: No file requires outside of /etc, /bin, /sbin, /usr/bin, /usr/sbin. [x]: Packager, Vendor, PreReq, Copyright tags should not be in spec file [x]: Sources can be downloaded from URI in Source: tag [x]: SourceX is a working URL. [x]: Spec use %global instead of %define unless justified. ===== EXTRA items ===== Generic: [x]: Rpmlint is run on all installed packages. Note: There are rpmlint messages (see attachment). [x]: Spec file according to URL is the same as in SRPM. fonts: [!]: Run repo-font-audit on all fonts in package. Note: Cannot find createrepo, install createrepo package to make a comprehensive font review. See: url: undefined [x]: Run ttname on all fonts in package. Note: ttname analyze results in fonts/ttname.log. Rpmlint ------- Checking: aftertheflood-spark-fonts-20170907-1.fc28.src.rpm aftertheflood-spark-fonts.src: W: spelling-error %description -l en_US Sparklines -> Spark lines, Spark-lines, Sparkles aftertheflood-spark-fonts.src: W: spelling-error %description -l en_US javascript -> java script, java-script, JavaScript 1 packages and 0 specfiles checked; 0 errors, 2 warnings. Rpmlint (installed packages) ---------------------------- Cannot parse rpmlint output: Requires -------- Provides -------- Source checksums ---------------- https://github.com/aftertheflood/spark/archive/ddbbf6f7b6cf9f1091b61748b6d44d1439d6f919/aftertheflood-spark-fonts-20170907.tar.gz : CHECKSUM(SHA256) this package : 902bbf34e352b75367a997a22fb509202152b19258ee2d32956c6d6f7ae80f5e CHECKSUM(SHA256) upstream package : 902bbf34e352b75367a997a22fb509202152b19258ee2d32956c6d6f7ae80f5e Generated by fedora-review 0.6.1 (f03e4e7) last change: 2016-05-02 Command line :/usr/bin/fedora-review -m fedora-rawhide-x86_64 -b 1492475 Buildroot used: fedora-rawhide-x86_64 Active plugins: Generic, fonts, Shell-api Disabled plugins: Java, C/C++, Python, SugarActivity, Ocaml, Perl, Haskell, R, PHP Disabled flags: EXARCH, DISTTAG, EPEL5, BATCH, EPEL6
In the metainfo.xml file, <metadata_license> is about the xml file itself. Since it's not code but content, it should probably be some CC-BY type thing (as it's written by you, feel free to choose) which seemed to be what's used by most font metadata files I looked at. Also, I wasn't exactly sure how to test this out. If I open the file using gnome-font-viewer, the display seems to be gibberish.
Thanks for your review! Fonts source are provided but they are compiled with makeotf, part of Adobe Font Development Kit for OpenType (AFDKO), a non free software: LICENSE Adobe grants You a non-exclusive, non-transferable, royalty-free license to use, reproduce, modify, and redistribute the items in this Package for the purposes of developing Developer Fonts. You agree not to reverse engineer, decompile, disassemble or otherwise attempt to discover the source code of the FDK except to the extent you may be expressly permitted to decompile under applicable law, it is essential to do so in order to achieve operability of the FDK with another software program, and you have first requested Adobe to provide the information necessary to achieve such operability and Adobe has not made such information available. Adobe has the right to impose reasonable conditions and to request a reasonable fee before providing such information. Any information supplied by Adobe or obtained by you, as permitted hereunder, may only be used by you for the purpose described herein. Adobe is under no obligation to provide any support under this Agreement, including upgrades or future versions of the FDK or other items in this Package, to Developer, end users, or to any other party. For Versionning, I followed this rule: >Do not trust font metadata versionning[4], unless you've checked upstream does >update versions on file changes. When in doubt use the timestamp of the most >recent font file as version, for example 20081231. > https://fedoraproject.org/wiki/Simple_fonts_spec_template Although font metadatas provide versions, these are not the same for every file, and if I choose the higher number, there's no guarantee a new release would just update one of the other font file. >The spec uses a date but pulls from a commit; this doesn't follow the snapshot versioning guidelines. Upstream didn't tag a release, beside saying "Here's version 1". I,ve asked them a Github release. >Speaking of licensing, MIT is not one of the recommended or explicitly approved licenses for fonts I don't think this is an issue, this page only list font licenses and the issue with GPL doesn't apply to MIT since it allows embedding. ("Good font licenses allow embedding"). >Since you were able to work with upstream to get the license added, you can suggest to them to add the "license description" (#13) or "license info URL" (#14) fields in the fonts themselves. I've queried upstream, hopefully they will accept my request.
>In the metainfo.xml file, <metadata_license> is about the xml file itself. Since it's not code but content, it should probably be some CC-BY type thing (as it's written by you, feel free to choose) which seemed to be what's used by most font metadata files I looked at. Noted and fixed, I'll post updated SPEC once upstream answer. >Also, I wasn't exactly sure how to test this out. If I open the file using gnome-font-viewer, the display seems to be gibberish. I've tested the font with LibreOffice Writer: select one of the font, and tyoe one of the example: 93.89{40,27,69,58,44,42,52,39,27,8,40,45,33,37,24,20,30,36,36,29,40,60,50,40,50,63,87,100}152.11 It should display bars, dots or lines.
(In reply to Robert-André Mauchin from comment #3) > Thanks for your review! > > Fonts source are provided but they are compiled with makeotf, part of Adobe > Font Development Kit for OpenType (AFDKO), a non free software. > OK, a comment would be good then. There is another discussion over on the hack-fonts review about using python-fontmake to script their builds; I'm not sure if this would be a possible avenue for upstream. And this is not yet packaged in Fedora, but soon. > > For Versionning, I followed this rule: > > >Do not trust font metadata versionning[4], unless you've checked upstream does > >update versions on file changes. When in doubt use the timestamp of the most > >recent font file as version, for example 20081231. > > > https://fedoraproject.org/wiki/Simple_fonts_spec_template > > Although font metadatas provide versions, these are not the same for every > file, and if I choose the higher number, there's no guarantee a new release > would just update one of the other font file. > Ah, I see; that seems to be a good practice for upstreams that just dump the result somewhere... > >The spec uses a date but pulls from a commit; this doesn't follow the snapshot versioning guidelines. > > Upstream didn't tag a release, beside saying "Here's version 1". I,ve asked > them a Github release. > ... but since they're using git, maybe they can take advantage of that to do proper tagging. > >Speaking of licensing, MIT is not one of the recommended or explicitly approved licenses for fonts > > I don't think this is an issue, this page only list font licenses and the > issue with GPL doesn't apply to MIT since it allows embedding. ("Good font > licenses allow embedding"). > I actually found the same list on the main licensing page as well: https://fedoraproject.org/wiki/Licensing:Main#Font_Licenses I'm inclined to agree with you, but since you have to wait for some correspondence from upstream, maybe you can confirm with legal@ in the meantime. Maybe the only downside is that MIT gives up more rights than font publishers traditionally like and there's really no effect on Fedora itself? (In reply to Robert-André Mauchin from comment #4) > >Also, I wasn't exactly sure how to test this out. If I open the file using gnome-font-viewer, the display seems to be gibberish. > > I've tested the font with LibreOffice Writer: select one of the font, and > tyoe one of the example: Ah, good to know LO supports these features. One more really small thing: the slash can be removed here: %install mkdir -p %{buildroot}/%{_fontdir}
I had no reply yet. Good thing is that they've dual licensed with SIL which a font license approved by Fedora.
The font has been renamed to Sparks and version 2.0 has been released. License has been added to the font metadata too. SPEC URL: https://raw.githubusercontent.com/eclipseo/packaging/8ecc6a5/aftertheflood-sparks-fonts.spec SRPM URL: https://copr-be.cloud.fedoraproject.org/results/eclipseo/aftertheflood-sparks/fedora-rawhide-x86_64/00711730-aftertheflood-sparks-fonts/aftertheflood-sparks-fonts-2.0-1.fc28.src.rpm SPEC diff: https://github.com/eclipseo/packaging/commit/8ecc6a50ac8e276fb7198962f8c64df64c120893#diff-e272db7fccba87eda45a3aa43baacdc1
Are not Bar and Dot separate families, and thus should be packaged separately? Can you still not build from source? I see some sources in the archive right now. There's also something wrong with either the metrics or LO, because if I type out the example using Sparks Bar Medium (and *some* of the other weights), the cursor ends up a little short (as does the highlight if I select everything.)
> Can you still not build from source? I see some sources in the archive right now. I can't build from source as I said before, they are compiled with makeotf, part of Adobe Font Development Kit for OpenType (AFDKO), a non free software. > Are not Bar and Dot separate families, and thus should be packaged separately? Yes, maybe, this is more complicated to implement. > There's also something wrong with either the metrics or LO, because if I type out the example using Sparks Bar Medium (and *some* of the other weights), the cursor ends up a little short (as does the highlight if I select everything.) I can't do much about this. SPEC URL: https://raw.githubusercontent.com/eclipseo/packaging/e8a2a48/aftertheflood-sparks-fonts.spec SRPM URL: https://copr-be.cloud.fedoraproject.org/results/eclipseo/aftertheflood-sparks/fedora-rawhide-x86_64/00713037-aftertheflood-sparks-fonts/aftertheflood-sparks-fonts-2.0-1.fc28.src.rpm SPEC diff: https://github.com/eclipseo/packaging/commit/e8a2a489ca56de1645e07cd8ba384af819ed48b5#diff-e272db7fccba87eda45a3aa43baacdc1
Looks good. APPROVED.
(fedrepo-req-admin): The Pagure repository was created at https://src.fedoraproject.org/rpms/aftertheflood-sparks-fonts. You may commit to the branch "f27" in about 10 minutes.
aftertheflood-sparks-fonts-2.0-1.fc27 has been submitted as an update to Fedora 27. https://bodhi.fedoraproject.org/updates/FEDORA-2018-bd6471dbac
aftertheflood-sparks-fonts-2.0-1.fc27 has been pushed to the Fedora 27 testing repository. If problems still persist, please make note of it in this bug report. See https://fedoraproject.org/wiki/QA:Updates_Testing for instructions on how to install test updates. You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2018-bd6471dbac
Please fill out the font description template [1] in the wiki for these new fonts as well. [1] https://fedoraproject.org/wiki/Font_description_template
aftertheflood-sparks-fonts-2.0-1.fc27 has been pushed to the Fedora 27 stable repository. If problems still persist, please make note of it in this bug report.