[Bug 1026376] Review Request: fontawesome-fonts - Iconic font set

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

 



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



--- Comment #3 from Ankur Sinha (FranciscoD) <sanjay.ankur@xxxxxxxxx> ---
[+] OK
[-] NA
[?] Issue

** Mandatory review guidelines: **
 [+] rpmlint output:
[asinha@ankur-laptop  SRPMS]$ rpmlint ../SPECS/fontawesome-fonts.spec
./fontawesome-fonts-4.0.0-1.fc20.src.rpm
/var/lib/mock/fedora-rawhide-x86_64/result/*.rpm
../SPECS/fontawesome-fonts.spec: W: invalid-url Source0:
http://fortawesome.github.io/Font-Awesome/assets/font-awesome-4.0.0.zip HTTP
Error 404: Not Found
fontawesome-fonts.src: W: spelling-error %description -l en_US scalable ->
salable, callable, calculable
fontawesome-fonts.src: W: invalid-url Source0:
http://fortawesome.github.io/Font-Awesome/assets/font-awesome-4.0.0.zip HTTP
Error 404: Not Found
fontawesome-fonts.noarch: W: spelling-error %description -l en_US scalable ->
salable, callable, calculable
fontawesome-fonts.noarch: W: no-documentation
fontawesome-fonts.src: W: spelling-error %description -l en_US scalable ->
salable, callable, calculable
fontawesome-fonts.src: W: invalid-url Source0:
http://fortawesome.github.io/Font-Awesome/assets/font-awesome-4.0.0.zip HTTP
Error 404: Not Found
fontawesome-fonts-web.noarch: W: no-documentation
4 packages and 1 specfiles checked; 0 errors, 8 warnings.
[asinha@ankur-laptop  SRPMS]$

^^ 
- A new version is out which should correct the 404 errors. 
- No documentation in src tarball. Please consider asking upstream to include a
  license if possible. Otherwise please remove the %doc macro since it's
  unneeded.

 [+] License is acceptable (...)
 [+] License field in spec is correct
^
License is included in the font metadata itself: SIL Open Font License 1.1

 [?] License files included in package %docs if included in source package
 [?] License files installed when any subpackage combination is installed
^
Please consider requesting upstream to include a license file. This isn't
necessary though, since the font clearly declares what license it's under.

 [+] Spec written in American English
 [+] Spec is legible
 [?] Sources match upstream unless altered to fix permissibility issues
   Upstream SHA256: ...
   Your SHA256:     ...
^ 
Cannot check md5 since the spec is using an older version which isn't
available. To be checked once spec is updated to pack new version.

 [+] Build succeeds on at least one primary arch
 [+] Build succeeds on all primary arches or has ExcludeArch + bugs filed
 [+] BuildRequires correct, justified where necessary
 [-] Locales handled with %find_lang, not %_datadir/locale/*
 [-] %post, %postun call ldconfig if package contains shared .so files
 [+] No bundled libs
 [-] Relocatability is justified
 [+] Package owns all directories it creates
 [+] Package requires others for directories it uses but does not own
 [+] No duplication in %files unless necessary for license files
 [+] File permissions are sane
 [+] Package contains permissible code or content
 [-] Large docs go in -doc subpackage
 [-] %doc files not required at runtime
 [-] Static libs go in -static package/virtual Provides
 [-] Development files go in -devel package
 [-] -devel packages Require base with fully-versioned dependency, %_isa
 [-] No .la files
 [-] GUI app uses .desktop file, installs it with desktop-file-install
 [+] File list does not conflict with other packages' without justification
 [+] File names are valid UTF-8

** Optional review guidelines: **
 [?] Query upstream about including license files
TODO

 [-] Translations of description, summary
 [-] Builds in mock
 [-] Builds on all arches
 [?] Functions as described (e.g. no crashes)
Not yet checked. Please do this.

 [+] Scriptlets are sane
 [+] Subpackages require base with fully-versioned dependency if sensible
 [-] .pc file subpackage placement is sensible
 [+] No file deps outside of /etc, /bin, /sbin, /usr/bin, /usr/sbin
 [-] Include man pages if available

Naming guidelines:
 [+] Package names use only a-zA-Z0-9-._+ subject to restrictions on -._+
 [+] Package names are sane
 [+] No naming conflicts
 [+] Spec file name matches base package name
 [+] Version is sane
 [+] Version does not contain ~
 [+] Release is sane
 [+] %dist tag
 [-] Case used only when necessary
 [-] Renaming handled correctly

Packaging guidelines:
 [+] Useful without external bits
 [-] No kmods
 [-] Pre-built binaries, libs removed in %prep
 [+] Sources contain only redistributable code or content
 [+] Spec format is sane
 [+] Package obeys FHS, except libexecdir, /run, /usr/target
 [+] No files in /bin, /sbin, /lib* on >= F17
 [-] Programs run before FS mounting use /run instead of /var/run
 [-] Binaries in /bin, /sbin do not depend on files in /usr on < F17
 [+] No files under /srv, /opt, /usr/local
 [+] Changelog in prescribed format
 [+] No Packager, Vendor, Copyright, PreReq tags
 [+] Summary does not end in a period
 [-] Correct BuildRoot tag on < EL6
 [-] Correct %clean section on < EL6
 [+] Requires correct, justified where necessary
 [+] Summary, description do not use trademarks incorrectly
 [-] All relevant documentation is packaged, appropriately marked with %doc
 [-] Doc files do not drag in extra dependencies (e.g. due to +x)
 [-] Code compilable with gcc is compiled with gcc
 [-] Build honors applicable compiler flags or justifies otherwise
 [-] PIE used for long-running/root daemons, setuid/filecap programs
 [-] Useful -debuginfo package or disabled and justified
 [-] Package with .pc files Requires pkgconfig on < EL6
 [-] No static executables
 [-] Rpath absent or only used for internal libs
 [+] Config files marked with %config(noreplace) or justified %config
 [+] No config files under /usr
 [-] Third party package manager configs acceptable, in %_docdir
 [-] .desktop files are sane
 [+] Spec uses macros consistently
 [+] Spec uses macros instead of hard-coded names where appropriate
 [-] Spec uses macros for executables only when configurability is needed
 [-] %makeinstall used only when alternatives don't work
 [-] Macros in Summary, description are expandable at srpm build time
 [+] Spec uses %{SOURCE#} instead of $RPM_SOURCE_DIR and %sourcedir
 [+] No software collections (scl)
 [-] Macro files named /etc/rpm/macros.%name
 [-] Build uses only python/perl/shell+coreutils/lua/BuildRequired langs
 [+] %global, not %define
 [-] Package translating with gettext BuildRequires it
 [-] Package translating with Linguist BuildRequires qt-devel
 [+] File ops preserve timestamps
 [-] Parallel make
 [+] No Requires(pre,post) notation
 [-] User, group creation handled correctly (See Packaging:UsersAndGroups)
 [-] Web apps go in /usr/share/%name, not /var/www
 [-] Conflicts are justified
 [+] One project per package
 [+] No bundled fonts
 [-] Patches have appropriate commentary
 [-] Available test suites executed in %check
 [-] tmpfiles.d used for /run, /run/lock on >= F15

File ownership list:
[asinha@ankur-laptop  result]$ for i in *rpm; do echo "== $i =="; rpmls "$i";
done
== fontawesome-fonts-4.0.0-1.fc21.noarch.rpm ==
lrwxrwxrwx  /etc/fonts/conf.d/60-fontawesome.conf
-rw-r--r--  /usr/share/fontconfig/conf.avail/60-fontawesome.conf
drwxr-xr-x  /usr/share/fonts/fontawesome
-rw-r--r--  /usr/share/fonts/fontawesome/FontAwesome.otf
-rw-r--r--  /usr/share/fonts/fontawesome/fontawesome-webfont.ttf
== fontawesome-fonts-4.0.0-1.fc21.src.rpm ==
-rw-rw-r--  font-awesome-4.0.0.zip
-rw-rw-r--  fontawesome-fonts-fontconfig.conf
-rw-rw-r--  fontawesome-fonts.spec
== fontawesome-fonts-web-4.0.0-1.fc21.noarch.rpm ==
drwxr-xr-x  /usr/share/font-awesome-4.0.0
drwxr-xr-x  /usr/share/font-awesome-4.0.0/css
-rw-r--r--  /usr/share/font-awesome-4.0.0/css/font-awesome.css
-rw-r--r--  /usr/share/font-awesome-4.0.0/css/font-awesome.min.css
drwxr-xr-x  /usr/share/font-awesome-4.0.0/less
-rw-r--r--  /usr/share/font-awesome-4.0.0/less/bordered-pulled.less
-rw-r--r--  /usr/share/font-awesome-4.0.0/less/core.less
-rw-r--r--  /usr/share/font-awesome-4.0.0/less/fixed-width.less
-rw-r--r--  /usr/share/font-awesome-4.0.0/less/font-awesome.less
-rw-r--r--  /usr/share/font-awesome-4.0.0/less/icons.less
-rw-r--r--  /usr/share/font-awesome-4.0.0/less/larger.less
-rw-r--r--  /usr/share/font-awesome-4.0.0/less/list.less
-rw-r--r--  /usr/share/font-awesome-4.0.0/less/mixins.less
-rw-r--r--  /usr/share/font-awesome-4.0.0/less/path.less
-rw-r--r--  /usr/share/font-awesome-4.0.0/less/rotated-flipped.less
-rw-r--r--  /usr/share/font-awesome-4.0.0/less/spinning.less
-rw-r--r--  /usr/share/font-awesome-4.0.0/less/stacked.less
-rw-r--r--  /usr/share/font-awesome-4.0.0/less/variables.less
drwxr-xr-x  /usr/share/font-awesome-4.0.0/scss
-rw-r--r--  /usr/share/font-awesome-4.0.0/scss/_bordered-pulled.scss
-rw-r--r--  /usr/share/font-awesome-4.0.0/scss/_core.scss
-rw-r--r--  /usr/share/font-awesome-4.0.0/scss/_fixed-width.scss
-rw-r--r--  /usr/share/font-awesome-4.0.0/scss/_icons.scss
-rw-r--r--  /usr/share/font-awesome-4.0.0/scss/_larger.scss
-rw-r--r--  /usr/share/font-awesome-4.0.0/scss/_list.scss
-rw-r--r--  /usr/share/font-awesome-4.0.0/scss/_mixins.scss
-rw-r--r--  /usr/share/font-awesome-4.0.0/scss/_path.scss
-rw-r--r--  /usr/share/font-awesome-4.0.0/scss/_rotated-flipped.scss
-rw-r--r--  /usr/share/font-awesome-4.0.0/scss/_spinning.scss
-rw-r--r--  /usr/share/font-awesome-4.0.0/scss/_stacked.scss
-rw-r--r--  /usr/share/font-awesome-4.0.0/scss/_variables.scss
-rw-r--r--  /usr/share/font-awesome-4.0.0/scss/font-awesome.scss
[asinha@ankur-laptop  result]$

Requires + Provides list:
[asinha@ankur-laptop  result]$ review-req-check
== fontawesome-fonts-4.0.0-1.fc21.noarch.rpm ==
Provides:
config(fontawesome-fonts) = 4.0.0-1.fc21
font(fontawesome)
fontawesome-fonts = 4.0.0-1.fc21

Requires:
/bin/sh
/bin/sh
config(fontawesome-fonts) = 4.0.0-1.fc21
fontpackages-filesystem

== fontawesome-fonts-4.0.0-1.fc21.src.rpm ==
Provides:

Requires:
fontpackages-devel

== fontawesome-fonts-web-4.0.0-1.fc21.noarch.rpm ==
Provides:
fontawesome-fonts-web = 4.0.0-1.fc21

Requires:
fontawesome-fonts = 4.0.0-1.fc21


- The fontconfig priority is OK. 
- The packaging is mostly OK. I just need to make a few more checks with the
updated version and I should be able to approve it.



- I see the fonts contain youtube etc in them. I think they're OK, and in line
with the trademarks, but you should check and consult LEGAL if necessary.

https://developers.google.com/youtube/branding
https://developer.linkedin.com/documents/branding-guidelines

- The font version according to font metadata is 3.2.0. Worth pointing out to
upstream and requesting them to update the metadata each time they make a new
release.

Thanks,
Warm regards,
Ankur

-- 
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]