[Bug 1228865] Review Request: gdouros-anaktoria-fonts - A font based on "Grecs du roi" and the "First Folio Edition of Shakespeare"

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

 



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

Zbigniew Jędrzejewski-Szmek <zbyszek@xxxxxxxxx> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
              Flags|fedora-review?              |fedora-review+



--- Comment #7 from Zbigniew Jędrzejewski-Szmek <zbyszek@xxxxxxxxx> ---
(In reply to Alexander Ploumistos from comment #6)
> (In reply to Zbigniew Jędrzejewski-Szmek from comment #5)
> > Where is the license specified?
> 
> There never was a license file. See here, bottom of the page:
So please add a comment to the spec file explaining this. I think you should
even
include the "license text" in that comment.

# https://web.archive.org/web/20150625020428/http://users.teilar.gr/~g1951d/
# "in lieu of a licence:
# Fonts and documents in this site are not pieces of property or merchandise
items;
# they carry no trademark, copyright, license or other market tags; they are
free
# for any use. George Douros"

> > %description seems to contain a private use unicode character (1480󿀄
> > 1561).
> 
> Thanks, there was a funny-looking zero, I fixed it in both the spec file and
> the metainfo.xml file. By the way, which tool picked that up?
I does not display properly in firefox on my system, and I started
investigating.
You probably have the right font installed.

> > [ ]: Each %files section contains %defattr if rpm < 4.4
> >      Note: %defattr present but not needed
> But I don't have a %defattr directive, where is this coming from?
Oh, indeed. It must be coming from one of the macros. I fixed
https://bugzilla.redhat.com/show_bug.cgi?id=1047031 some time ago.
I'm not sure where this one came from. Please ignore, it's not a bug
in your package anyway.

> > [ ]: Large documentation must go in a -doc subpackage. Large could be size
> >      (~1MB) or number of files.
> >      Note: Documentation size is 808960 bytes in 1 files.
> > That's borderline. A bit too small to create a separate package.
> 
> See comments 1 & 4 here:
> 
> https://bugzilla.redhat.com/show_bug.cgi?id=1208842
Yeah, agreed.

> > Appdata file should be validated in %check
> > [https://fedoraproject.org/wiki/Packaging:AppData].
> 
> Does this apply to metainfo.xml files? I thought it was just for the
> appdata.xml ones.
Yes. That paragraph talks about both kinds of files, and says that files should
be checked with making a distinction between the two types.

> > $ appstream-util validate-relax
> > /usr/share/appdata/gdouros-anaktoria.metainfo.xml
> > /usr/share/appdata/gdouros-anaktoria.metainfo.xml: FAILED:
> > • markup-invalid        : <id> does not have correct extension for kind
> > • tag-missing           : <extends> is not present
> > Validation of files failed
> 
> On an F22 system, I'm getting this:
> $ appstream-util validate-relax
> rpmbuild/SOURCES/gdouros-anaktoria.metainfo.xml 
> rpmbuild/SOURCES/gdouros-anaktoria.metainfo.xml: OK
> 
> I can't understand why there would be a problem with the id tag or why the
> extends tag would be needed, it does not extend anything.
> 
> On what system did you run fedora-review?
I run that on F21. On rawhide indeed it doesn't say anything.

> I've just noticed that fedora-review on this system creates an F21 package
> even though I fed it an F23 source rpm built in mock, is there a setting
> someplace that I've missed?
Most likely you have /etc/mock/default.cfg linked to fedora-21-x86_64.cfg.
I link it to fedora-rawhide-x86_64.cfg instead.

--

To sum up, please add:
- a comment about the license
- %check with appstream-util validate-relax

Package is APPROVED.

Are all Douros fonts packaged? If you have any left to package, I'll be happy
to 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@xxxxxxxxxxxxxxxxxxxxxxx
https://admin.fedoraproject.org/mailman/listinfo/package-review




[Index of Archives]     [Fedora Legacy]     [Fedora Desktop]     [Fedora SELinux]     [Yosemite News]     [KDE Users]     [Fedora Tools]