Bug 1079090

Summary: Review Request: layla-fonts - A collection of traditional Arabic fonts
Product: [Fedora] Fedora Reporter: Mohammed Isam <mohammed_isam1984>
Component: Package ReviewAssignee: Jonathan Dieter <jdieter>
Status: CLOSED ERRATA QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: medium Docs Contact:
Priority: medium    
Version: rawhideCC: empateinfinito, fonts-bugs, i18n-bugs, jdieter, package-review, panemade
Target Milestone: ---Keywords: Reopened
Target Release: ---Flags: jdieter: fedora-review+
gwync: fedora-cvs+
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: layla-fonts-1.4-1.el6 Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2015-10-18 23:48:55 UTC Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:
Attachments:
Description Flags
ttname.log
none
repo-font-audit.log none

Description Mohammed Isam 2014-03-21 00:38:36 UTC
Spec URL: http://sites.google.com/site/mohammedisam2000/home/projects/layla-fonts.spec
SRPM URL: http://sites.google.com/site/mohammedisam2000/home/projects/layla-fonts-1.0-1.fc20.src.rpm
Description: This package is a collection of traditional arabic fonts (including Thuluth, Koufi, Ruqaa..) in addition to other newly designed fonts. The aim is to provide all the basic fonts an arabic user will need under X window system. More fonts will be added regularly to the collection to make it the only font source an arabic user will need to install under the X window system.
Fedora Account System Username: mohammedisam

Comment 1 Christopher Meng 2014-03-21 01:54:16 UTC
1. Are you going to support EPEL5?

If not remove these:

Group:		User Interface/X

BuildRoot:	%(mktemp -ud %{_tmppath}/%{name}-%{version}-%{release}-XXXXXX)

rm -fr %{buildroot} in %install

%clean section

%defattr(0664,root,root,0755)

2. install -D man/man1/layla.1 %{buildroot}%{_mandir}/man1/layla.1

missing -p option again.

3. install -D info/layla.info %{buildroot}%{_infodir}/layla.info

Comment 2 Mohammed Isam 2014-03-21 02:31:31 UTC
(In reply to Christopher Meng from comment #1)
> 1. Are you going to support EPEL5?
> 
> If not remove these:
> 
> Group:		User Interface/X
> 
> BuildRoot:	%(mktemp -ud %{_tmppath}/%{name}-%{version}-%{release}-XXXXXX)
> 
> rm -fr %{buildroot} in %install
> 
> %clean section
> 
> %defattr(0664,root,root,0755)
> 
> 2. install -D man/man1/layla.1 %{buildroot}%{_mandir}/man1/layla.1
> 
> missing -p option again.
> 
> 3. install -D info/layla.info %{buildroot}%{_infodir}/layla.info

1. No, not supporting EPEL5. Edited the spec file as noted.
2. & 3. Added the -p otion.
Spec file is updated. Please review.

Comment 3 Mohammed Isam 2014-07-11 12:51:41 UTC
Added font wiki page:
https://fedoraproject.org/wiki/Layla_fonts

Comment 5 Mohammed Isam 2014-07-17 18:18:20 UTC
Any review for this request please?

Comment 7 Mohammed Isam 2014-07-17 19:30:16 UTC
Added a koji scratch build:
http://koji.fedoraproject.org/koji/taskinfo?taskID=7159906

Comment 8 Jonathan Dieter 2015-01-22 09:30:18 UTC
Are you still interested in having this reviewed?

Comment 9 Mohammed Isam 2015-01-22 14:14:12 UTC
(In reply to Jonathan Dieter from comment #8)
> Are you still interested in having this reviewed?

Definitely! This will be wonderful. Thanx a lot man.

Comment 10 Carlos Morel-Riquelme 2015-02-06 05:20:56 UTC
Hello Mohammed i hope that my info can be useful, well you have this issues

Checking: layla-koufi-fonts-1.1-1.fc22.noarch.rpm
          layla-thuluth-fonts-1.1-1.fc22.noarch.rpm
          layla-boxer-fonts-1.1-1.fc22.noarch.rpm
          layla-ruqaa-fonts-1.1-1.fc22.noarch.rpm
          layla-basic-arabic-fonts-1.1-1.fc22.noarch.rpm
          layla-fonts-common-1.1-1.fc22.noarch.rpm
          layla-fonts-1.1-1.fc22.src.rpm
layla-koufi-fonts.noarch: W: no-documentation
layla-thuluth-fonts.noarch: W: no-documentation
layla-boxer-fonts.noarch: W: no-documentation
layla-ruqaa-fonts.noarch: W: no-documentation
layla-basic-arabic-fonts.noarch: W: no-documentation
layla-fonts-common.noarch: W: spurious-executable-perm /usr/share/man/man1/layla.1.gz
layla-fonts-common.noarch: W: spurious-executable-perm /usr/share/info/layla.info.gz
layla-fonts.src: W: spelling-error Summary(en_US) arabic -> Arabic
layla-fonts.src: W: spelling-error %description -l en_US arabic -> Arabic
7 packages and 0 specfiles checked; 0 errors, 9 warnings.

The W: no-documentation warning is because the GPL license is missing, please create a GPL.txt file and later copy the license into this.
Now, add the GPL+.txt to the layla-fonts folder and later compress this.

For solved the :W spelling-error please change "arabic" to "Arabic" in your Summary and the %description in the spec file.

About the W: spurious-executable warning please read this
http://fedoraproject.org/wiki/MinGW/Rpmlint#spurious-executable-perm


I hope my english can be understandable.

Regards from Chile

Comment 11 Mohammed Isam 2015-02-06 08:29:19 UTC
Thanx a lot Carlos
I corrected the no-documentation warnings (I added %files sections for each package), and the exec problem, and the arabic is now Arabic!
Thanx

Comment 13 Carlos Morel-Riquelme 2015-02-06 08:54:07 UTC
Mohammed, please check you spec file

look this:
Version: 1.1

change to:
Version: 1.2

----------------------------------------------------------------------------

look this:

%changelog
* Wed Jul 16 2014 Mohammed Isam <mohammed_isam1984> 1.1-1
- Updated info and manpages

add to changelog:

%changelog
* Fri Feb 06 2015 Mohammed Isam <mohammed_isam1984> 1.2-1
- Fix and update the spec file

-------------------------------------------------------------------------------

If you see your *.srpm have 1.2 version and the spec file have 1.1 version.

Regards :)

Comment 14 Jonathan Dieter 2015-02-06 12:00:16 UTC
Mohammad, the problem is that the spec link still points to the old spec file.  I've downloaded the srpm and run some preliminary checks.  Thanks for fixing the items Carlos listed, but you also have %files being listed twice for each subpackage because %_font_pkg automatically creates a %files section for a font.

Comment 15 Mohammed Isam 2015-02-06 14:12:31 UTC
(In reply to Carlos Morel-Riquelme from comment #13)
> Mohammed, please check you spec file
> 
> look this:
> Version: 1.1
> 
> change to:
> Version: 1.2
> 
> ----------------------------------------------------------------------------
> 

Hi Carlos, this is my mistake. I uploaded the new spec file, but google didn't update the old file, instead it saved a new copy and reserved the old one. I fixed it. The link points to the correct file now.

Comment 16 Mohammed Isam 2015-02-06 14:16:40 UTC
(In reply to Jonathan Dieter from comment #14)
> Mohammad, the problem is that the spec link still points to the old spec
> file.  I've downloaded the srpm and run some preliminary checks.  Thanks for
> fixing the items Carlos listed, but you also have %files being listed twice
> for each subpackage because %_font_pkg automatically creates a %files
> section for a font.

Hi Jonathan
I am a little perplexed here. If I remove the %files sections, rpmbuild complains about finding (Installed but unpackaged files) and terminates in error. Still, it is working with the current spec file, how is it possible? Furthermore, when I run (rpm -qpl) on the font packages, it lists the files correctly (with no repeated files). Should I remove the %files sections from my spec? If yes, how to solve the rpmbuild panic error?

Comment 17 Carlos Morel-Riquelme 2015-02-06 15:01:32 UTC
Rpmlint
-------
Checking: layla-koufi-fonts-1.2-1.fc22.noarch.rpm
          layla-thuluth-fonts-1.2-1.fc22.noarch.rpm
          layla-boxer-fonts-1.2-1.fc22.noarch.rpm
          layla-ruqaa-fonts-1.2-1.fc22.noarch.rpm
          layla-basic-arabic-fonts-1.2-1.fc22.noarch.rpm
          layla-fonts-common-1.2-1.fc22.noarch.rpm
          layla-fonts-1.2-1.fc22.src.rpm
layla-fonts.src: E: specfile-error warning: line 108: second %files
layla-fonts.src: E: specfile-error warning: line 114: second %files
layla-fonts.src: E: specfile-error warning: line 120: second %files
layla-fonts.src: E: specfile-error warning: line 126: second %files
layla-fonts.src: E: specfile-error warning: line 132: second %files
7 packages and 0 specfiles checked; 5 errors, 0 warnings.

it's strange because in the past this errors don't exists, please compare the old spec file with the new spec file. Other idea is that is errors have relation with that johathan says.

Regards

Comment 18 Jonathan Dieter 2015-02-06 17:34:12 UTC
(In reply to Mohammed Isam from comment #16)
> (In reply to Jonathan Dieter from comment #14)
> > Mohammad, the problem is that the spec link still points to the old spec
> > file.  I've downloaded the srpm and run some preliminary checks.  Thanks for
> > fixing the items Carlos listed, but you also have %files being listed twice
> > for each subpackage because %_font_pkg automatically creates a %files
> > section for a font.
> 
> Hi Jonathan
> I am a little perplexed here. If I remove the %files sections, rpmbuild
> complains about finding (Installed but unpackaged files) and terminates in
> error. Still, it is working with the current spec file, how is it possible?
> Furthermore, when I run (rpm -qpl) on the font packages, it lists the files
> correctly (with no repeated files). Should I remove the %files sections from
> my spec? If yes, how to solve the rpmbuild panic error?

The problem is that the manpages aren't packaged by %_font_pkg but are listed in the %files sections.  I would suggest dropping both the man and info pages as they're really not necessary for a font.  I checked a couple of other Fedora fonts and they don't contain man or info pages.

If you remove the man and info pages, then you can also remove all of the %files sections except -common

Comment 19 Mohammed Isam 2015-02-06 18:06:49 UTC
(In reply to Jonathan Dieter from comment #18)
> The problem is that the manpages aren't packaged by %_font_pkg but are
> listed in the %files sections.  I would suggest dropping both the man and
> info pages as they're really not necessary for a font.  I checked a couple
> of other Fedora fonts and they don't contain man or info pages.
> 
> If you remove the man and info pages, then you can also remove all of the
> %files sections except -common

Ok. I removed the man pages & the info files. Links are updated accordingly.

Comment 21 Mohammed Isam 2015-02-06 18:08:16 UTC
NB: I had to bump the NVR as I removed the info & mans from the source pkg.

Comment 22 Carlos Morel-Riquelme 2015-02-07 07:55:56 UTC
Hi mohammed 
The fedora-review give me again the warning of documentation

Rpmlint
-------
Checking: layla-koufi-fonts-1.3-1.fc22.noarch.rpm
          layla-thuluth-fonts-1.3-1.fc22.noarch.rpm
          layla-boxer-fonts-1.3-1.fc22.noarch.rpm
          layla-ruqaa-fonts-1.3-1.fc22.noarch.rpm
          layla-basic-arabic-fonts-1.3-1.fc22.noarch.rpm
          layla-fonts-common-1.3-1.fc22.noarch.rpm
          layla-fonts-1.3-1.fc22.src.rpm
layla-koufi-fonts.noarch: W: no-documentation
layla-thuluth-fonts.noarch: W: no-documentation
layla-boxer-fonts.noarch: W: no-documentation
layla-ruqaa-fonts.noarch: W: no-documentation
layla-basic-arabic-fonts.noarch: W: no-documentation
7 packages and 0 specfiles checked; 0 errors, 5 warnings.

--------------------------------------------------------------------------------

Here is the tree of the layla-fonts in the srpm

[empateinfinito@localhost layla-fonts-1.3]$ tree
.
├── Authors
├── ChangeLog
├── confs
│   ├── 67-layla-BasicArabic.conf
│   ├── 67-layla-Boxer.conf
│   ├── 67-layla-Koufi.conf
│   ├── 67-layla-Ruqaa.conf
│   └── 67-layla-Thuluth.conf
├── COPYING
├── GPL.txt
├── LaylaBasicArabic.ttf
├── LaylaBoxer.ttf
├── layla-fonts-1.3.tar.gz
├── LaylaKoufi.ttf
├── LaylaRuqaa.ttf
├── LaylaThuluth.ttf
└── README

1 directory, 16 files
[empateinfinito@localhost layla-fonts-1.3]$ 

1) It's strange because the documentation is here :/

2) layla-fonts-1.3.tar.gz is void ?

Comment 23 Jonathan Dieter 2015-02-07 08:45:59 UTC
(In reply to Carlos Morel-Riquelme from comment #22)
> Hi mohammed 
> The fedora-review give me again the warning of documentation

<snip>

> 1) It's strange because the documentation is here :/
> 
> 2) layla-fonts-1.3.tar.gz is void ?

The documentation is there for the -common subpackage, it just doesn't get included for each individual font.  This follows the precedent set by other fonts in Fedora.  

$ rpm -ql liberation-fonts-common-1.07.4-4.fc21.noarch
/etc/X11/fontpath.d/liberation-fonts
/usr/share/appdata/liberation.metainfo.xml
/usr/share/doc/liberation-fonts-common
/usr/share/doc/liberation-fonts-common/AUTHORS
/usr/share/doc/liberation-fonts-common/COPYING
/usr/share/doc/liberation-fonts-common/ChangeLog
/usr/share/doc/liberation-fonts-common/License.txt
/usr/share/doc/liberation-fonts-common/README
/usr/share/doc/liberation-fonts-common/TODO
/usr/share/fonts/liberation
/usr/share/fonts/liberation/fonts.dir
/usr/share/fonts/liberation/fonts.scale

$ rpm -ql liberation-mono-fonts-1.07.4-4.fc21.noarch
/etc/fonts/conf.d/59-liberation-mono.conf
/usr/share/appdata/liberation-mono.metainfo.xml
/usr/share/fontconfig/conf.avail/59-liberation-mono.conf
/usr/share/fonts/liberation
/usr/share/fonts/liberation/LiberationMono-Bold.ttf
/usr/share/fonts/liberation/LiberationMono-BoldItalic.ttf
/usr/share/fonts/liberation/LiberationMono-Italic.ttf
/usr/share/fonts/liberation/LiberationMono-Regular.ttf

Note that the -mono subpackage in the Liberation fonts has no documentation.  This warning can be ignored.

Comment 24 Jonathan Dieter 2015-02-07 08:52:52 UTC
Carlos, are you wanting to do this review?  If not, I'll go ahead and do the full review.  If you're looking for experience in reviewing packages, I'll happily let you take the lead on this and just look over your shoulder.

Comment 25 Carlos Morel-Riquelme 2015-02-07 23:10:57 UTC
Hello jonathan please take the control, i'm a newbie in reviews so i've will very happy to learn about this review and also of your knowledge.

my best regards from Chile :)

Comment 26 Mohammed Isam 2015-02-08 04:04:16 UTC
(In reply to Carlos Morel-Riquelme from comment #25)
> Hello jonathan please take the control, i'm a newbie in reviews so i've will
> very happy to learn about this review and also of your knowledge.
> 
> my best regards from Chile :)

Many thanx 4 u both. I appreciate it.

Comment 27 Parag AN(पराग) 2015-02-08 10:03:53 UTC
Carlos,
  We don't want to duplicate documentation(license) files when one of the sub-package installs it and base (or any other) package needs this sub-package. In this case we get only one copy of documentation files from sub-package.

See https://fedoraproject.org/wiki/Packaging:LicensingGuidelines#Subpackage_Licensing

So, when any of layla-*-fonts sub-package we install, as we have written 
Requires: %{name}-common = %{version}-%{release}
layla-common will also get installed which will give us documentation files.

We can then ignore rpmlint message "no-documentation".

Comment 28 Mohammed Isam 2015-02-08 18:08:55 UTC
(In reply to Parag AN(पराग) from comment #27)
> So, when any of layla-*-fonts sub-package we install, as we have written 
> Requires: %{name}-common = %{version}-%{release}
> layla-common will also get installed which will give us documentation files.
> 
> We can then ignore rpmlint message "no-documentation".

Thanx man. It was really confusing in the start but I think this clears everything up nicely!

Comment 29 Jonathan Dieter 2015-02-09 14:11:07 UTC
Rpmlint
-------
Checking: layla-koufi-fonts-1.3-1.fc21.noarch.rpm
          layla-thuluth-fonts-1.3-1.fc21.noarch.rpm
          layla-boxer-fonts-1.3-1.fc21.noarch.rpm
          layla-ruqaa-fonts-1.3-1.fc21.noarch.rpm
          layla-basic-arabic-fonts-1.3-1.fc21.noarch.rpm
          layla-fonts-common-1.3-1.fc21.noarch.rpm
          layla-fonts-1.3-1.fc21.src.rpm
layla-koufi-fonts.noarch: W: no-documentation
layla-thuluth-fonts.noarch: W: no-documentation
layla-boxer-fonts.noarch: W: no-documentation
layla-ruqaa-fonts.noarch: W: no-documentation
layla-basic-arabic-fonts.noarch: W: no-documentation
7 packages and 0 specfiles checked; 0 errors, 5 warnings.

Comment 30 Jonathan Dieter 2015-02-09 14:11:27 UTC
Requires
--------
layla-basic-arabic-fonts (rpmlib, GLIBC filtered):
    /bin/sh
    config(layla-basic-arabic-fonts)
    layla-fonts-common

layla-koufi-fonts (rpmlib, GLIBC filtered):
    /bin/sh
    config(layla-koufi-fonts)
    layla-fonts-common

layla-ruqaa-fonts (rpmlib, GLIBC filtered):
    /bin/sh
    config(layla-ruqaa-fonts)
    layla-fonts-common

layla-boxer-fonts (rpmlib, GLIBC filtered):
    /bin/sh
    config(layla-boxer-fonts)
    layla-fonts-common

layla-fonts-common (rpmlib, GLIBC filtered):

layla-thuluth-fonts (rpmlib, GLIBC filtered):
    /bin/sh
    config(layla-thuluth-fonts)
    layla-fonts-common

Comment 31 Jonathan Dieter 2015-02-09 14:11:49 UTC
Provides
--------
layla-basic-arabic-fonts:
    config(layla-basic-arabic-fonts)
    font(laylabasicarabic)
    layla-basic-arabic-fonts

layla-koufi-fonts:
    config(layla-koufi-fonts)
    font(:lang=ar)
    font(laylakoufi)
    layla-koufi-fonts

layla-ruqaa-fonts:
    config(layla-ruqaa-fonts)
    font(:lang=ar)
    font(laylaruqaa)
    layla-ruqaa-fonts

layla-boxer-fonts:
    config(layla-boxer-fonts)
    font(:lang=ar)
    font(laylaboxer)
    layla-boxer-fonts

layla-fonts-common:
    layla-fonts-common

layla-thuluth-fonts:
    config(layla-thuluth-fonts)
    font(laylathuluth)
    layla-thuluth-fonts

Comment 32 Jonathan Dieter 2015-02-09 14:12:12 UTC
Source checksums
----------------
http://sites.google.com/site/mohammedisam2000/home/projects/layla-fonts-1.3.tar.gz :
  CHECKSUM(SHA256) this package     : 0217e7fb48e34057488b267ef17aa91fbb9e97b2bda52e6a8ccf7f06020df422
  CHECKSUM(SHA256) upstream package : 0217e7fb48e34057488b267ef17aa91fbb9e97b2bda52e6a8ccf7f06020df422

Comment 33 Jonathan Dieter 2015-02-09 14:12:54 UTC
Created attachment 989702 [details]
ttname.log

Comment 34 Jonathan Dieter 2015-02-09 14:13:56 UTC
Created attachment 989703 [details]
repo-font-audit.log

Comment 35 Jonathan Dieter 2015-02-09 14:17:01 UTC
Package Review
==============

Legend:
[x] = Pass, [!] = Fail, [-] = Not applicable


===== MUST items =====

Generic:
[!]: Package must own all directories that it creates.
     Note: Directories without known owners: /usr/share/fonts,
     /etc/fonts/conf.d, /usr/share/fontconfig/conf.avail,
     /usr/share/fontconfig, /etc/fonts

     Please add Requires: fontpackages-filesystem to the -common subpackage

[!]: Each %files section contains %defattr if rpm < 4.4

     Note: %defattr present but not needed

[!]: License field in the package spec file matches the actual license.

     Package license and license included in tarball is GPLv3+, but, according 
     to ttname.log, licenses in ttf files are all SIL OFL 1.1.  Since you're 
     also upstream for this package, I'd recommend sticking with SIL OFL 1.1.
     
[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 file installed when any subpackage combination is installed.
[x]: Package contains no bundled libraries without FPC exception.
[x]: Changelog in prescribed format.
[x]: Sources contain only permissible code or content.
[x]: 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.
[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]: 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 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]: %config files are marked noreplace or the reason is justified.
[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]: No %config files under /usr.
[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
[-]: Development files must be in a -devel package
[-]: Large documentation must go in a -doc subpackage. Large could be size
     (~1MB) or number of files.
     Note: Documentation size is 51200 bytes in 4 files.
[-]: If the package is a rename of another package, proper Obsoletes and
     Provides are present.

===== SHOULD items =====

Generic:
[x]: Final provides and requires are sane (see attachments).
[x]: Package functions as described.
[x]: Latest version is packaged.
[x]: Package does not include license text files separate from upstream.
[x]: Package should compile and build into binary rpms on all supported
     architectures.
[x]: Packages should try to preserve timestamps of original installed files.
[x]: Packager, Vendor, PreReq, Copyright tags should not be in spec file
[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]: 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 %define unless justified.
[-]: If the source package does not include license text(s) as a separate file
     from upstream, the packager SHOULD query upstream to include it.
[-]: Scriptlets must be sane, if used.
[-]: Description and summary sections in the package spec file contains
     translations for supported Non-English languages, if available.
[-]: %check is present and all tests pass.

===== EXTRA items =====

Generic:
[x]: Rpmlint is run on all installed packages.
     Note: There are rpmlint messages (see comment #29).
[x]: Spec file according to URL is the same as in SRPM.

fonts:
[!]: Run repo-font-audit on all fonts in package.
     
     layla-koufi, layla-boxer and layla-thuluth are missing some glyphs in the
     Arabic block.  This is not a blocker, but it would be nice to have this
     fixed.

     Note: full results in attached repo-font-audit.log.

[x]: Run ttname on all fonts in package.
     Note: ttname analyze results in attached ttname.log.

Comment 36 Mohammed Isam 2015-02-09 17:24:04 UTC
(In reply to Jonathan Dieter from comment #35)
> Package Review
> ==============
> [!]: Package must own all directories that it creates.
>      Note: Directories without known owners: /usr/share/fonts,
>      /etc/fonts/conf.d, /usr/share/fontconfig/conf.avail,
>      /usr/share/fontconfig, /etc/fonts
> 
>      Please add Requires: fontpackages-filesystem to the -common subpackage
> 

I added the Requires line to the spec. I don't need to add the individual ownerships to the %files directory, do I?

> [!]: Each %files section contains %defattr if rpm < 4.4
> 
>      Note: %defattr present but not needed

Not fixed. Honestly I don't get it. My %files section doesn't contain any %defattr?

> 
> [!]: License field in the package spec file matches the actual license.
> 
>      Package license and license included in tarball is GPLv3+, but,
> according 
>      to ttname.log, licenses in ttf files are all SIL OFL 1.1.  Since you're 
>      also upstream for this package, I'd recommend sticking with SIL OFL 1.1.
>      

Added the OFL license file (actually, three files according to the OFL directions). Removed the GPL file and updated the License field in the spec.

Comment 38 Jonathan Dieter 2015-02-09 17:44:11 UTC
===== MUST items =====

[x]: Package must own all directories that it creates.

     This is fixed now.

[x]: Each %files section contains %defattr if rpm < 4.4

     Apologies!  This was brought up by fedora-review and I took it at its word.
     I assume that the %_font_pkg macro uses %defattr.  Either way, feel free to
     ignore this.

[x]: License field in the package spec file matches the actual license.

     Looks good!

===== EXTRA items =====

[!]: Run repo-font-audit on all fonts in package.
     
     layla-koufi, layla-boxer and layla-thuluth are missing some glyphs in the
     Arabic block.  This *isn't* a blocker, but if you could keep it in mind (or
     just tell me that repo-font-audit doesn't know what it's talking about) that
     would be great.

     Note: full results in attached repo-font-audit.log.

Comment 39 Jonathan Dieter 2015-02-09 17:44:45 UTC
This package is APPROVED!

Comment 40 Mohammed Isam 2015-02-09 19:28:26 UTC
(In reply to Jonathan Dieter from comment #38)
> 
> [!]: Run repo-font-audit on all fonts in package.
>      
>      layla-koufi, layla-boxer and layla-thuluth are missing some glyphs in
> the Arabic block.  This *isn't* a blocker, but if you could keep it in mind
> (or just tell me that repo-font-audit doesn't know what it's talking about)
> that would be great.
> 

Yep. Some glyphs are missing (Latin ones), but as the font is mainly covering the Arabic range, all the important glyphs for correct Arabic display are included. This is going to be fixed in the next releases.

Comment 41 Mohammed Isam 2015-02-09 19:32:02 UTC
(In reply to Jonathan Dieter from comment #39)
> This package is APPROVED!

Thanx a lot Jonathan :)

Comment 42 Mohammed Isam 2015-02-09 19:35:13 UTC
New Package SCM Request
=======================
Package Name: layla-fonts
Short Description: A collection of traditional Arabic fonts
Upstream URL: http://sites.google.com/site/mohammedisam/layla-fonts
Owners: mohammedisam
Branches: f20 f21 f22 el5 el6 epel7
InitialCC:

Comment 43 Gwyn Ciesla 2015-02-09 20:06:05 UTC
Git done (by process-git-requests).

Comment 44 Fedora Update System 2015-02-13 18:34:37 UTC
layla-fonts-1.4-1.el7 has been submitted as an update for Fedora EPEL 7.
https://admin.fedoraproject.org/updates/layla-fonts-1.4-1.el7

Comment 45 Fedora Update System 2015-02-13 18:35:23 UTC
layla-fonts-1.4-1.el6 has been submitted as an update for Fedora EPEL 6.
https://admin.fedoraproject.org/updates/layla-fonts-1.4-1.el6

Comment 46 Fedora Update System 2015-02-13 18:37:11 UTC
layla-fonts-1.4-1.fc20 has been submitted as an update for Fedora 20.
https://admin.fedoraproject.org/updates/layla-fonts-1.4-1.fc20

Comment 47 Fedora Update System 2015-02-13 18:37:42 UTC
layla-fonts-1.4-1.fc21 has been submitted as an update for Fedora 21.
https://admin.fedoraproject.org/updates/layla-fonts-1.4-1.fc21

Comment 48 Fedora Update System 2015-02-14 02:42:38 UTC
layla-fonts-1.4-1.el7 has been pushed to the Fedora EPEL 7 testing repository.

Comment 49 Fedora Update System 2015-02-23 23:25:45 UTC
layla-fonts-1.4-1.fc21 has been pushed to the Fedora 21 stable repository.

Comment 50 Fedora Update System 2015-02-23 23:26:15 UTC
layla-fonts-1.4-1.fc20 has been pushed to the Fedora 20 stable repository.

Comment 51 Fedora Update System 2015-03-08 22:43:20 UTC
layla-fonts-1.4-1.el7 has been pushed to the Fedora EPEL 7 stable repository.

Comment 52 Fedora Update System 2015-03-08 22:48:13 UTC
layla-fonts-1.4-1.el6 has been pushed to the Fedora EPEL 6 stable repository.

Comment 53 Fedora Update System 2015-10-08 14:32:14 UTC
layla-fonts-1.5-1.fc23 has been submitted as an update to Fedora 23. https://bodhi.fedoraproject.org/updates/FEDORA-2015-0912c62798

Comment 54 Fedora Update System 2015-10-08 14:32:59 UTC
layla-fonts-1.5-1.fc22 has been submitted as an update to Fedora 22. https://bodhi.fedoraproject.org/updates/FEDORA-2015-82c7240959

Comment 55 Fedora Update System 2015-10-08 14:33:49 UTC
layla-fonts-1.5-1.fc21 has been submitted as an update to Fedora 21. https://bodhi.fedoraproject.org/updates/FEDORA-2015-fe27a4fb6b

Comment 56 Fedora Update System 2015-10-08 14:35:18 UTC
layla-fonts-1.5-1.el6 has been submitted as an update to Fedora EPEL 6. https://bodhi.fedoraproject.org/updates/FEDORA-EPEL-2015-1ef24fcd30

Comment 57 Fedora Update System 2015-10-08 14:36:04 UTC
layla-fonts-1.5-1.el7 has been submitted as an update to Fedora EPEL 7. https://bodhi.fedoraproject.org/updates/FEDORA-EPEL-2015-e36655974d

Comment 58 Fedora Update System 2015-10-09 12:51:40 UTC
layla-fonts-1.5-1.fc21 has been pushed to the Fedora 21 testing repository. If problems still persist, please make note of it in this bug report.
If you want to test the update, you can install it with
$ su -c 'dnf --enablerepo=updates-testing update layla-fonts'
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2015-fe27a4fb6b

Comment 59 Fedora Update System 2015-10-09 13:54:54 UTC
layla-fonts-1.5-1.fc22 has been pushed to the Fedora 22 testing repository. If problems still persist, please make note of it in this bug report.
If you want to test the update, you can install it with
$ su -c 'dnf --enablerepo=updates-testing update layla-fonts'
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2015-82c7240959

Comment 60 Fedora Update System 2015-10-09 13:55:43 UTC
layla-fonts-1.5-1.fc23 has been pushed to the Fedora 23 testing repository. If problems still persist, please make note of it in this bug report.
If you want to test the update, you can install it with
$ su -c 'dnf --enablerepo=updates-testing update layla-fonts'
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2015-0912c62798

Comment 61 Fedora Update System 2015-10-09 22:53:47 UTC
layla-fonts-1.5-1.el7 has been pushed to the Fedora EPEL 7 testing repository. If problems still persist, please make note of it in this bug report.
If you want to test the update, you can install it with
$ su -c 'yum --enablerepo=epel-testing update layla-fonts'
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-EPEL-2015-e36655974d

Comment 62 Fedora Update System 2015-10-11 05:50:56 UTC
layla-fonts-1.5-1.el6 has been pushed to the Fedora EPEL 6 testing repository. If problems still persist, please make note of it in this bug report.
If you want to test the update, you can install it with
$ su -c 'yum --enablerepo=epel-testing update layla-fonts'
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-EPEL-2015-1ef24fcd30

Comment 63 Fedora Update System 2015-10-18 23:48:52 UTC
layla-fonts-1.5-1.fc22 has been pushed to the Fedora 22 stable repository. If problems still persist, please make note of it in this bug report.

Comment 64 Fedora Update System 2015-10-19 04:20:21 UTC
layla-fonts-1.5-1.fc21 has been pushed to the Fedora 21 stable repository. If problems still persist, please make note of it in this bug report.

Comment 65 Fedora Update System 2015-10-28 16:25:50 UTC
layla-fonts-1.5-1.el6 has been pushed to the Fedora EPEL 6 stable repository. If problems still persist, please make note of it in this bug report.

Comment 66 Fedora Update System 2015-10-29 14:53:12 UTC
layla-fonts-1.5-1.el7 has been pushed to the Fedora EPEL 7 stable repository. If problems still persist, please make note of it in this bug report.

Comment 67 Fedora Update System 2015-11-01 02:44:28 UTC
layla-fonts-1.5-1.fc23 has been pushed to the Fedora 23 stable repository. If problems still persist, please make note of it in this bug report.