Bug 860249 - Review Request: adobe-source-code-pro-fonts - A set of mono-spaced OpenType fonts designed for coding environments
Summary: Review Request: adobe-source-code-pro-fonts - A set of mono-spaced OpenType f...
Keywords:
Status: CLOSED DUPLICATE of bug 1007615
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
unspecified
medium
Target Milestone: ---
Assignee: Nobody's working on this, feel free to take it
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2012-09-25 11:48 UTC by Tobias Florek
Modified: 2015-07-21 12:28 UTC (History)
23 users (show)

Fixed In Version:
Clone Of:
: 1007615 (view as bug list)
Environment:
Last Closed: 2014-02-18 15:42:38 UTC
Type: ---
Embargoed:


Attachments (Terms of Use)
diff of spec file from adobe-source-sans-pro-fonts (2.04 KB, patch)
2012-09-27 13:17 UTC, Matt Rose
no flags Details | Diff

Description Tobias Florek 2012-09-25 11:48:37 UTC
Spec URL: http://www.math.hu-berlin.de/~florek/adobe-sourcecodepro-font.spec
SRPM URL: http://www.math.hu-berlin.de/~florek/adobe-sourcecodepro-fonts-1.009-1.fc17.src.rpm
Description: 
This font was designed by Paul D. Hunt as a companion to Source Sans. It has
the same weight range as the corresponding Source Sans design.  It supports
a wide range of languages using the Latin script, and includes all the
characters in the Adobe Latin 4 glyph set.

Fedora Account System Username: ibotty

Comment 1 Tobias Florek 2012-09-25 12:13:48 UTC
oh, i noticed i got the title wrong. it should have been
adobe-sourcecodepro-fonts. no need to create a new bug, right?

Comment 2 Michael J Gruber 2012-09-25 13:19:28 UTC
(In reply to comment #1)
> oh, i noticed i got the title wrong. it should have been
> adobe-sourcecodepro-fonts. no need to create a new bug, right?

You can edit the bug summary. I just did that for you.

Also, the spec file should be named the same way as the package ;)

Have you tried building the fonts from source? It's not a must, only should.

Comment 3 Tobias Florek 2012-09-25 13:31:34 UTC
ah i knew i forgot something. i made a mental note to tell the reviewer, that unfortunately the font uses proprietary adobe tools to build. i doubt a fedora buildroot can or should be made to build them.

re the spec's name. that was a typo :D.

Comment 4 Michael J Gruber 2012-09-25 13:50:44 UTC
(In reply to comment #3)
> ah i knew i forgot something. i made a mental note to tell the reviewer,
> that unfortunately the font uses proprietary adobe tools to build. i doubt a
> fedora buildroot can or should be made to build them.

I didn't check, but if AFDKO is proprietary and we don't have the tools to build from source then packaging just the fonts is the way to go.

> re the spec's name. that was a typo :D.

Sure.

Rebuilding with a renamed spec file gives an rpmlint-clean package.

The font packaging policy template has been followed.
(Note you'll have to register your package in comps once it's in.)

Installing the package and using the fonts works as expected (LibreWriter).

I can't do a complete formal review because you're not a packager yet and you need a "sponsor" to formally review your first package. I'm just a packager.

Other than that, you're ready to go. Congrats!

Comment 5 Tobias Florek 2012-09-25 14:27:02 UTC
thanks for the review. it has been very easy to package. the rpm macros are great.

seems i forgot to run rpmlint against the spec file. against the rpm there was no warning either way.

i set the bug to depend on FE-NEEDSPONSOR.

Comment 6 Josh Stone 2012-09-25 16:16:19 UTC
Since adobe-source-sans-pro-fonts was added in bug #845743, I think you should at least follow the same naming style for yours, i.e. adobe-source-code-pro-fonts.  Maybe you can even collaborate with Alexis since these are so closely related.

However, I'm also not a sponsor, sorry, nor am I familiar with font packaging.

Comment 7 Tobias Florek 2012-09-25 17:24:15 UTC
hi josh,

i did follow _my_ naming style (see bug #860316). sorry for not checking in the bugzilla about the already packaged font using the other naming style.

new SRPM http://www.math.hu-berlin.de/~florek/adobe-source-code-pro-fonts-1.009-2.fc17.src.rpm
new SPEC http://www.math.hu-berlin.de/~florek/adobe-source-code-pro-font.spec

Comment 8 Matt Rose 2012-09-26 18:46:45 UTC
I built the same rpm, probably for much the same reason as Tobias.  I Just took the SPEC file from the existing adobe-source-sans-pro-fonts rpm and edited the pertinent lines and rebuilt against the package downloaded from Sourceforge

SPEC: https://github.com/mattrose/Source-Code-Pro/blob/master/rpm/SPECS/adobe-source-code-pro-fonts.spec
SRPM: https://github.com/mattrose/Source-Code-Pro/blob/master/rpm/SRPMS/adobe-source-code-pro-fonts-1.009-1.fc17.src.rpm
RPM: https://github.com/mattrose/Source-Code-Pro/blob/master/rpm/RPMS/noarch/adobe-source-code-pro-fonts-1.009-1.fc17.noarch.rpm

All my terminals have it and it has saved me a bunch of eye-strain already.  It really is a very nice font.

Comment 9 Michael J Gruber 2012-09-27 10:08:09 UTC
(In reply to comment #7)

If you were aware of the dupe for source sans it would have been nice to say so here.

Also, your URLs give "object not found".

(In reply to comment #8)
> I built the same rpm, probably for much the same reason as Tobias.  I Just
> took the SPEC file from the existing adobe-source-sans-pro-fonts rpm and
> edited the pertinent lines and rebuilt against the package downloaded from
> Sourceforge
> 
> SPEC:
> https://github.com/mattrose/Source-Code-Pro/blob/master/rpm/SPECS/adobe-
> source-code-pro-fonts.spec
> SRPM:
> https://github.com/mattrose/Source-Code-Pro/blob/master/rpm/SRPMS/adobe-
> source-code-pro-fonts-1.009-1.fc17.src.rpm
> RPM:
> https://github.com/mattrose/Source-Code-Pro/blob/master/rpm/RPMS/noarch/
> adobe-source-code-pro-fonts-1.009-1.fc17.noarch.rpm
> 
> All my terminals have it and it has saved me a bunch of eye-strain already. 
> It really is a very nice font.

rpmlint gives 2 errors on that rpm. The lines in the spec are too long, and the source URL does not resolve.

Is this really a straight adaption of the source sans spec?

Have you filed a packaging request, Matt? We don't want another dupe.

Comment 10 Tobias Florek 2012-09-27 10:25:05 UTC
i was not aware of the dupe when i started. i will not take offence on the tone in your reply.

the fixed the spec url (it does only differ in the package name anyway):
http://www.math.hu-berlin.de/~florek/adobe-source-code-pro-fonts.spec

Comment 11 Michael J Gruber 2012-09-27 11:45:05 UTC
(In reply to comment #10)
> i was not aware of the dupe when i started. i will not take offence on the
> tone in your reply.

?? There would have been no reason to.

> the fixed the spec url (it does only differ in the package name anyway):
> http://www.math.hu-berlin.de/~florek/adobe-source-code-pro-fonts.spec

Thanks.

I compared your spec to the one for adobe source sans pro. The points where they differ don't matter too much (your use of %{fontname} is actually more complete) except for one which I missed first: Unless there's a specific reason, you should package only one format of the font, in this case only otf, see

https://fedoraproject.org/wiki/Shipping_fonts_in_Fedora_%28FAQ%29#What_if_my_package_bundles_the_same_font_in_several_different_formats.3F

So, please remove the 2 occurrences of \*.ttf and increase the -2 to -3. (It's good you did that during review for -1 to -2 already, but please update the changelog also.)

Everything else still looks good.

Comment 12 Matt Rose 2012-09-27 12:50:08 UTC
(In reply to comment #9)
> (In reply to comment #7)
> 
> 
> rpmlint gives 2 errors on that rpm. The lines in the spec are too long, and
> the source URL does not resolve.

I'll take a look at that.

> 
> Is this really a straight adaption of the source sans spec?

Yep, just changed Sans to Code in a few places.  I'll attach the diff once I fix the rpmlint errors.

> 
> Have you filed a packaging request, Matt? We don't want another dupe.

No, I found this packaging request and noticed that Tobias's URLs weren't resolving, so I added the url's to this bug for the packages that I had already built.

Comment 13 Matt Rose 2012-09-27 13:17:58 UTC
Created attachment 618050 [details]
diff of spec file from adobe-source-sans-pro-fonts

Here's the versions that are all fixed up by rpmlint

SPEC: https://raw.github.com/mattrose/Source-Code-Pro/master/rpm/SPECS/adobe-source-code-pro-fonts.spec
RPM: https://github.com/mattrose/Source-Code-Pro/raw/master/rpm/RPMS/noarch/adobe-source-code-pro-fonts-1.009-1.fc17.noarch.rpm
SRPM: https://github.com/mattrose/Source-Code-Pro/raw/master/rpm/SRPMS/adobe-source-code-pro-fonts-1.009-1.fc17.src.rpm

rpmlint output:


[mattrose@oscar SPECS]$ rpmlint adobe-source-code-pro-fonts.spec ../SRPMS/adobe-source-code-pro-fonts-1.009-1.fc17.src.rpm ../RPMS/noarch/adobe-source-code-pro-fonts-1.009-1.fc17.noarch.rpm 
2 packages and 1 specfiles checked; 0 errors, 0 warnings.

Comment 14 Michael J Gruber 2012-09-27 13:24:47 UTC
(In reply to comment #12)
> (In reply to comment #9)
> > (In reply to comment #7)
> > 
> > 
> > rpmlint gives 2 errors on that rpm. The lines in the spec are too long, and
> > the source URL does not resolve.
> 
> I'll take a look at that.
> 
> > 
> > Is this really a straight adaption of the source sans spec?
> 
> Yep, just changed Sans to Code in a few places.  I'll attach the diff once I
> fix the rpmlint errors.
> 

An srpm built from the current sans package repo in fedora gives no errors, btw.
 
> > Have you filed a packaging request, Matt? We don't want another dupe.
> 
> No, I found this packaging request and noticed that Tobias's URLs weren't
> resolving, so I added the url's to this bug for the packages that I had
> already built.

Note that "raw" URLs would have been more helpful. I'm probably not the only one who uses save link/wget/curl/etc on them just to get github's html/xml.

Since Tobias was the first to submit the package in bz I suggest giving him the time to polish his spec. My informal review is there already, but he needs a sponsor and he's been turning the appropriate bz wheels already, which is good.

Comment 15 Matt Rose 2012-09-27 13:41:27 UTC
No problem.  I just want the package in, so I don't have to maintain it on my own.  if I could ask you guys to keep the name the same as I have (adobe-source-code-pro-fonts) I would appreciate it.

Comment 16 Tobias Florek 2012-09-28 07:52:27 UTC
hi,

spec: http://www.math.hu-berlin.de/~florek/adobe-source-code-pro-fonts.spec
srpm: http://www.math.hu-berlin.de/~florek/adobe-source-code-pro-fonts-1.009-3.fc17.src.rpm

but honestly, i don't care what package gets chosen. if a known packager's package comes first, no problem. i can send diffs when a new version comes out anyway.

Comment 17 Matt Rose 2012-09-28 13:43:45 UTC
(In reply to comment #16)
> hi,
> 
> but honestly, i don't care what package gets chosen. if a known packager's
> package comes first, no problem. i can send diffs when a new version comes
> out anyway.

I'm not a known packager either, I'm willing to maintain the package going forward, as it is the only really nice monospace font I've been able to find for Fedora, and fonts matter.

But I've only started the process of becoming a fedora contributor by following the steps outlined in the wiki, basically I just need a sponsor at this point, so either way is fine.

Comment 18 Matt Rose 2012-09-28 13:45:50 UTC
(In reply to comment #14)
> (In reply to comment #12)
> > (In reply to comment #9)
> > > (In reply to comment #7)

> 
> Note that "raw" URLs would have been more helpful. I'm probably not the only
> one who uses save link/wget/curl/etc on them just to get github's html/xml.
> 


Yeah, I realized that I didn't post the raw URLs from github the first time.  I fixed that in comment #13

Comment 19 joshua 2012-09-28 15:23:18 UTC
Love it!  Lets add Source Sans Pro too... http://www.google.com/webfonts/specimen/Source+Sans+Pro

Comment 20 Tobias Florek 2012-09-30 09:09:42 UTC
look for adobe-source-sans-pro-fonts.noarch in the repo.

Comment 22 Matt Rose 2012-10-03 12:49:09 UTC
I was able to upgrade to the rpm built from the srpm in comment #21 from my custom one in comment #13 with no ill effect.

Hopefully this makes it in soon!

Comment 23 Christopher Meng 2012-10-24 01:48:54 UTC
I’m looking forward to seeing this package in repo...

Comment 24 Pete Travis 2012-11-17 16:46:52 UTC
Hey Tobias,

I have a couple informal comments to offer:

The package and spec file pass rpmlint, and the font looks great!

I don't see where I can download Source1, %{name}-fontconfig.conf . Is this publicly available?

You have `rm -fr %{buildroot}` in both %install and %clean. You shouldn't have to do this in either section.

You are missing a %files section. The package should claim ownership of every file it installs here. By convention, %doc should also be in this section.

Comment 25 Susi Lehtola 2012-11-17 17:20:04 UTC
(In reply to comment #24)
> You have `rm -fr %{buildroot}` in both %install and %clean. You shouldn't
> have to do this in either section.

Although this is done automatically by newer versions of RPM, it's in no sense wrong to do it.
 
> You are missing a %files section. The package should claim ownership of
> every file it installs here. By convention, %doc should also be in this
> section.

Nope, doesn't apply to fonts. See

https://fedoraproject.org/wiki/Packaging:FontsPolicy
https://fedoraproject.org/wiki/Simple_fonts_spec_template

The %files section is already provided by the %_font_pkg macro.

Comment 26 Tobias Florek 2012-11-18 14:00:12 UTC
jussi already answered a few things. the adobe-source-code-pro-fonts-fontconfig.conf file is (of course) included in the srpm, but i uploaded it for your convinience. new spec and srpm (and fontconfig.conf) at

 http://www.math.hu-berlin.de/~florek/adobe-source-code-pro-fonts.spec
 http://www.math.hu-berlin.de/~florek/adobe-source-code-pro-fonts-1.010-2.fc18.src.rpm
 http://www.math.hu-berlin.de/~florek/adobe-source-code-pro-fonts-fontconfig.conf

Comment 27 Rick Elrod 2012-11-24 05:41:06 UTC
I'm not a sponsor, so I can't do an official review, but until someone does, I threw these in a Fedora People repo for F17-18 and EL6, because I needed them anyway and figured someone else might too.

http://repos.fedorapeople.org/repos/codeblock/adobe-source-code-pro-fonts/

I'll probably take the repo down after this lands in the official repos, so I've set the .repo file to enabled=0, meaning you'll have to explicitly enable this to install them, just so when I take it down it doesn't hurt anyone.

To use:
Install the appropriate .repo file and `sudo yum install adobe-source-code-pro-fonts --enablerepo=fedora-adobe-source-code-pro-fonts`

Comment 29 Matt Rose 2012-12-07 14:08:45 UTC
Can we get some movement from Fedora on this feature?  Tobias opened it over two months ago, and he's shown great devotion to keeping the package up-to-date over the last two months.

What are the next steps that have to happen before this is accepted into Fedora?  

Is there a current package maintainer willing to sponsor this?

Comment 30 Michael J Gruber 2012-12-07 14:22:02 UTC
(In reply to comment #29)
> Can we get some movement from Fedora on this feature?  Tobias opened it over
> two months ago, and he's shown great devotion to keeping the package
> up-to-date over the last two months.
> 
> What are the next steps that have to happen before this is accepted into
> Fedora?  
> 
> Is there a current package maintainer willing to sponsor this?

Since Tobias is not a packager yet, he needs a sponsor in order to become a packager. Then he can file the request to create the package in the infrastructure.

I'd be happy to help out, but I'm just a packager myself, not a sponsor. Please try to find a sponsor on the fedora-devel list, saying you want to become a packager, need a sponsor etc. Tobias may need to do a few informal package reviews before becoming a packager (in order to prove he's aware of the guidelines) in addition to having specced his own package.

Alternatively, a packager could take over this package from Tobias right away to get it in, but that's not the idea behind the process.

Comment 31 Matthew Miller 2012-12-07 20:08:34 UTC
(In reply to comment #30)
> Please try to find a sponsor on the fedora-devel list, saying you want to
> become a packager, need a sponsor etc. Tobias may need to do a few informal
> package reviews before becoming a packager (in order to prove he's aware of
> the guidelines) in addition to having specced his own package.

I'm going to post to the fonts sig mailing list pointing to this bug. It seems like a font packager would be the most appropriate sponsor here.

Comment 33 Michael Schwendt 2013-01-21 16:16:49 UTC
> %clean
> %_font_pkg -f %{fontconf} *.otf

Suspicious and misleading.

The %clean here ought to be deleted, because the line below it has nothing to do with it. The %_font_pkg is a macro specific to fonts packages, and it starts a %files section.

Comment 34 Tobias Florek 2013-01-21 16:38:44 UTC
thanks for noticing. ironically that was a leftover from cleanup that %clean ended up empty.

new uploads:
 http://www.math.hu-berlin.de/~florek/adobe-source-code-pro-fonts.spec
 http://www.math.hu-berlin.de/~florek/adobe-source-code-pro-fonts-1.017-2.fc18.src.rpm

Comment 35 Michael Schwendt 2013-01-21 16:43:47 UTC
Tobias, at which step of the following procedure are you?
https://fedoraproject.org/wiki/Join_the_package_collection_maintainers

Comment 36 Tobias Florek 2013-01-21 16:57:53 UTC
basically i need a sponser.

but in detail. missing is:

- Inform Upstream (for this font)
- introduce yourself
- Get Sponsored
and the following steps.

i might get to write the two mails today.

Comment 37 Michael Schwendt 2013-01-21 20:45:00 UTC
The https://fedoraproject.org/wiki/Font_package_lifecycle page suggests that the reviewer/sponsor has "good knowledge of font packaging policies". It also mentions the "repo-font-audit" tool, which complains about a couple of things. It would be good if the repo-font-audit report would be examined and commented on, also in case some of the findings are false positives. [ for comparison, the adobe-source-sans-pro-fonts review request was bug 845743 but it does not mention repo-font-audit ]

Comment 38 Tobias Florek 2013-01-21 21:43:19 UTC
i'll look into repo-font-audit later.

for now: a diff between the other bug's adobe-source-sans-pro-fonts.spec and my adobe-source-code-pro.spec does not yield many functional differences, does it?

is there anything in particular that you want me to look at?

Comment 39 Tobias Florek 2013-01-21 22:00:11 UTC
(i really like repo-font-audit's use of unicode)

the following is a commented copy of every warning that repo-font-audit reports.


— Warning: bad font naming
  
  Source Code Pro, Semibold → Source Code Pro, DemiBold
    SourceCodePro-Semibold.otf	[adobe-source-code-pro-fonts]

i don't know what to do here. the explanation says that i should add a fontconfig rule and try to fix it upstream.


— Warning: fonts that do not pass fontlint sanity checks
  [adobe-source-code-pro-fonts](7)
  ⇒ 7 file(s) (0.596004 MiB) in 1 package(s) (0.263145 MiB) generated from 1 source
  package(s).

— Suggestion: fonts with partial script coverage
  [adobe-source-code-pro-fonts](7)
  ⇒ 7 file(s) (0.596004 MiB) in 1 package(s) (0.263145 MiB) generated from 1 source
  package(s).

— Suggestion: fonts with partial unicode block coverage
  [adobe-source-code-pro-fonts](7)
  ⇒ 7 file(s) (0.596004 MiB) in 1 package(s) (0.263145 MiB) generated from 1 source
  package(s).

should i raise this upstream? i'm sure they know about unicode coverage and i don't expect they will fix the fontlint's sanity checks without a human-noticable report.

Comment 40 Nicolas Mailhot 2013-01-22 15:05:46 UTC
All the repo-font-audit warnings should be notified upstream. They don't use the same tools as us under Linux, so they won't necessarily be aware of the problems we find.

The naming issue is the touchy one. Ordinarily I'd just ask upstream to just apply the canonical font naming defined by Microsoft, but I suspect Adobe may have another naming standard they are pushing. Anyway it'd be interesting to know why they diverge from the naming defined in the WWS whitepaper (it may be they have access to a more recent version of this document, it'd be good to get our hands on this).

Comment 41 Matt Rose 2013-01-22 17:46:13 UTC
Tobias, I would just open GitHub issues with these warnings from repo-font-audit.   The team that monitor github tickets seem open to improvements.

Comment 42 Tobias Florek 2013-04-09 10:40:55 UTC
sorry for not doing this earlier. i was busy the last months. i expect to be busy for this week as well. hopefully i'll get to it next week.

thanks for the pointer to github. i'll raise them as github tickets.

Comment 43 Matt Rose 2013-05-21 17:02:00 UTC
(In reply to Tobias Florek from comment #42)
> sorry for not doing this earlier. i was busy the last months. i expect to be
> busy for this week as well. hopefully i'll get to it next week.
> 
> thanks for the pointer to github. i'll raise them as github tickets.

I've become impatient ( a personal failing ) to get this package into fedora sooner rather than later.  I sent an email to Tobias, but I haven't got a response yet, so I'm going to try and get a sponsor myself for this package, and if Tobias ever wants to pick it up, I'd be perfectly willing to have him as a co-maintainer.

I'll also check github and see if the repo-font-audit issues have been raised

Comment 44 Dan Mashal 2013-09-09 16:59:14 UTC
Not even sure how I grabbed this but I'll try and do a review for you. I cannot sponsor though, will defer to rdieter on that one.

Comment 45 Matt Rose 2013-09-09 17:34:17 UTC
(In reply to Dan Mashal from comment #44)
> Not even sure how I grabbed this but I'll try and do a review for you. I
> cannot sponsor though, will defer to rdieter on that one.

Honestly I have almost given up on ever finding a sponsor.  I was going to send out one more intro to the list after my vacation at the end of this month, but I've kind of given up on this ever making it into fedora, which is too bad because it is a nice font, and one I use all over the place, with the packages from this bug

Comment 46 Rex Dieter 2013-09-09 17:51:41 UTC
Matt, I'd be happy to sponsor you per Dan's plan.

Comment 47 joshua 2013-09-09 17:57:21 UTC
Fedora people... it shouldn't be this hard.  Please fix this process!

Comment 48 Rex Dieter 2013-09-09 18:01:20 UTC
Joshua, the blocker here isn't fedora at this moment.

The original submitter isn't responding lately, and Matt offerred to submit something else as a replacement (but hasn't yet).  Until *someone* submits something reviewable, the process isn't what is broken.

Comment 49 Tobias Florek 2013-09-09 18:52:12 UTC
i'm sorry for not pursuing that further, but i didn't have much time lately and i don't think that will change that year.

there is quiet a bit to do.

  * development moved from sourceforge to github, so i am pretty sure i missed a few updates.
 https://github.com/adobe/source-code-pro

  * and if i understood the lwn article last (?) week correctly (i only skimmed it, so i might be wrong), it should now be possible to build the font with open source tools, so the packaging has to be remade as well.

Comment 50 Matt Rose 2013-09-10 01:14:20 UTC
Hey Tobias, I don't mind taking it over at all if you don't have time, I've been tracking the package on github.

I also read that lwn article (here for those interested https://lwn.net/Articles/564803/), but I don't see any changes in the docs or build scripts on github, so, though I would love to redo the spec file to incorporate those changes, they're not (publically) available yet.

I'm going to open up a new bug with my own submission, at which time I'll resolve this one as a dupe.  Tobias, thanks for all your work on getting this going.

Comment 51 Dan Mashal 2013-09-11 09:20:38 UTC
There is a lot of noise with this review so I'm just going to get this over with.


Naming: OK (cant find any dupes with files or naming)
Licensing: OK (OFL/SIL)
Build on rawhide: http://koji.fedoraproject.org/koji/taskinfo?taskID=5921976
Build on f19: http://koji.fedoraproject.org/koji/taskinfo?taskID=5921978
Build on el6: https://koji.fedoraproject.org/koji/taskinfo?taskID=5921980

Installing the package was successful.

Not too sure how to test.

fedora-review results:


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.
[ ]: License field in the package spec file matches the actual license.
     Note: Checking patched sources after %prep for licenses. No licenses
     found. Please check the source files for licenses manually.
[ ]: Package contains no bundled libraries without FPC exception.
[ ]: Changelog in prescribed format.
[ ]: Sources contain only permissible code or content.
[ ]: Package contains desktop file if it is a GUI application.
[ ]: Development files must be in a -devel package
[ ]: Package uses nothing in %doc for runtime.
[ ]: Package consistently uses macros (instead of hard-coded directory names).
[ ]: Package is named according to the Package Naming Guidelines.
[ ]: Package does not generate any conflict.
[ ]: Package obeys FHS, except libexecdir and /usr/target.
[ ]: If the package is a rename of another package, proper Obsoletes and
     Provides are present.
[ ]: Requires correct, justified where necessary.
[ ]: Spec file is legible and written in American English.
[ ]: Package contains systemd file(s) if in need.
[ ]: Package is not known to require an ExcludeArch tag.
[ ]: Large documentation must go in a -doc subpackage. Large could be size
     (~1MB) or number of files.
     Note: Documentation size is 20480 bytes in 2 files.
[ ]: 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 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]: Each %files section contains %defattr if rpm < 4.4
[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]: 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

===== 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.
[ ]: Final provides and requires are sane (see attachments).
[ ]: Package functions as described.
[ ]: Latest version is packaged.
[ ]: 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.
[ ]: 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]: Package has no %clean section with rm -rf %{buildroot} (or
     $RPM_BUILD_ROOT)
[x]: Dist tag is present (not strictly required in GL).
[x]: No file requires outside of /etc, /bin, /sbin, /usr/bin, /usr/sbin.
[x]: Fully versioned dependency in subpackages if applicable.
[x]: SourceX tarball generation or download is documented.
[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: No rpmlint messages.
[x]: Large data in /usr/share should live in a noarch subpackage if package is
     arched.
[x]: Spec file according to URL is the same as in SRPM.


Rpmlint
-------
Checking: adobe-source-code-pro-fonts-1.017-2.fc19.noarch.rpm
          adobe-source-code-pro-fonts-1.017-2.fc19.src.rpm
adobe-source-code-pro-fonts.src: E: specfile-error warning: bogus date in %changelog: Tue Nov 18 2012 Tobias Florek me - 1.010-2
2 packages and 0 specfiles checked; 1 errors, 0 warnings.




Rpmlint (installed packages)
----------------------------
# rpmlint adobe-source-code-pro-fonts
1 packages and 0 specfiles checked; 0 errors, 0 warnings.
# echo 'rpmlint-done:'



Requires
--------
adobe-source-code-pro-fonts (rpmlib, GLIBC filtered):
    /bin/sh
    config(adobe-source-code-pro-fonts)
    fontpackages-filesystem



Provides
--------
adobe-source-code-pro-fonts:
    adobe-source-code-pro-fonts
    config(adobe-source-code-pro-fonts)
    font(:lang=aa)
    font(:lang=af)
    font(:lang=an)
    font(:lang=ast)
    font(:lang=ay)
    font(:lang=az-az)
    font(:lang=bi)
    font(:lang=bin)
    font(:lang=br)
    font(:lang=bs)
    font(:lang=ca)
    font(:lang=ch)
    font(:lang=co)
    font(:lang=crh)
    font(:lang=cs)
    font(:lang=csb)
    font(:lang=cy)
    font(:lang=da)
    font(:lang=de)
    font(:lang=en)
    font(:lang=eo)
    font(:lang=es)
    font(:lang=et)
    font(:lang=eu)
    font(:lang=fi)
    font(:lang=fil)
    font(:lang=fj)
    font(:lang=fo)
    font(:lang=fr)
    font(:lang=fur)
    font(:lang=fy)
    font(:lang=gd)
    font(:lang=gl)
    font(:lang=gn)
    font(:lang=gv)
    font(:lang=haw)
    font(:lang=ho)
    font(:lang=hr)
    font(:lang=hsb)
    font(:lang=ht)
    font(:lang=hu)
    font(:lang=ia)
    font(:lang=id)
    font(:lang=ie)
    font(:lang=ig)
    font(:lang=io)
    font(:lang=is)
    font(:lang=it)
    font(:lang=jv)
    font(:lang=ki)
    font(:lang=kj)
    font(:lang=kl)
    font(:lang=ku-tr)
    font(:lang=kwm)
    font(:lang=la)
    font(:lang=lb)
    font(:lang=li)
    font(:lang=lt)
    font(:lang=lv)
    font(:lang=mg)
    font(:lang=mh)
    font(:lang=ms)
    font(:lang=mt)
    font(:lang=na)
    font(:lang=nb)
    font(:lang=nds)
    font(:lang=ng)
    font(:lang=nl)
    font(:lang=nn)
    font(:lang=no)
    font(:lang=nr)
    font(:lang=nso)
    font(:lang=nv)
    font(:lang=ny)
    font(:lang=oc)
    font(:lang=om)
    font(:lang=pap-an)
    font(:lang=pap-aw)
    font(:lang=pl)
    font(:lang=pt)
    font(:lang=qu)
    font(:lang=rm)
    font(:lang=rn)
    font(:lang=ro)
    font(:lang=rw)
    font(:lang=sc)
    font(:lang=sg)
    font(:lang=shs)
    font(:lang=sk)
    font(:lang=sl)
    font(:lang=sm)
    font(:lang=sma)
    font(:lang=smj)
    font(:lang=sn)
    font(:lang=so)
    font(:lang=sq)
    font(:lang=ss)
    font(:lang=st)
    font(:lang=su)
    font(:lang=sv)
    font(:lang=sw)
    font(:lang=tk)
    font(:lang=tl)
    font(:lang=tn)
    font(:lang=to)
    font(:lang=tr)
    font(:lang=ts)
    font(:lang=ty)
    font(:lang=uz)
    font(:lang=vi)
    font(:lang=vo)
    font(:lang=vot)
    font(:lang=wa)
    font(:lang=wen)
    font(:lang=xh)
    font(:lang=yap)
    font(:lang=za)
    font(:lang=zu)
    font(sourcecodepro)
    font(sourcecodeproblack)
    font(sourcecodeproextralight)
    font(sourcecodeprolight)
    font(sourcecodepromedium)
    font(sourcecodeprosemibold)



Source checksums
----------------
http://downloads.sourceforge.net/project/sourcecodepro.adobe/SourceCodePro_FontsOnly-1.017.zip :
  CHECKSUM(SHA256) this package     : 8136b4686309c428ef073356ab178c2f7e8f7b6fadd5a6c61b6a20646377b21f
  CHECKSUM(SHA256) upstream package : 8136b4686309c428ef073356ab178c2f7e8f7b6fadd5a6c61b6a20646377b21f


Generated by fedora-review 0.5.0 (920221d) last change: 2013-08-30
Command line :/usr/bin/fedora-review -b 860249
Buildroot used: fedora-19-x86_64
Active plugins: Generic, Shell-api
Disabled plugins: Java, C/C++, Python, SugarActivity, Perl, R, PHP, Ruby
Disabled flags: EPEL5, EXARCH, DISTTAG


Non blockers:

$ rpmlint ./adobe-source-code-pro-fonts-1.017-2.fc18.src.rpm 
adobe-source-code-pro-fonts.src: E: specfile-error warning: bogus date in %changelog: Tue Nov 18 2012 Tobias Florek me - 1.010-2
1 packages and 0 specfiles checked; 1 errors, 0 warnings.

$ rpmlint ./adobe-source-code-pro-fonts.spec 
./adobe-source-code-pro-fonts.spec: E: specfile-error warning: bogus date in %changelog: Tue Nov 18 2012 Tobias Florek me - 1.010-2
0 packages and 1 specfiles checked; 1 errors, 0 warnings.



APPROVED, please fix the changelog before import. I'm deferring the rest of this to Rex and will contact him on IRC to review my review (and make me look bad).

Comment 52 Christopher Meng 2013-09-11 09:40:53 UTC
APPROVED for what?

You don't have the power to sponsor him.

Comment 53 Dan Mashal 2013-09-11 23:10:38 UTC
(In reply to Christopher Meng from comment #52)
> APPROVED for what?

I approved the package review itself and for rdieter to sponsor him.

--

Per conversation with rdieter on IRC

The new plan now is for Matt to take over for Tobias and submit a new review request, and close this request as a duplicate.

Comment 54 Christopher Meng 2013-09-11 23:32:34 UTC
(In reply to Dan Mashal from comment #53)
> (In reply to Christopher Meng from comment #52)
> > APPROVED for what?
> 
> I approved the package review itself and for rdieter to sponsor him.
> 
> --
> 
> Per conversation with rdieter on IRC
> 
> The new plan now is for Matt to take over for Tobias and submit a new review
> request, and close this request as a duplicate.

I can only see Matt's comment here, however if you push this review further, it would be great. 

Thanks for your help.

Comment 55 Paul Flo Williams 2014-02-18 15:42:38 UTC
This review was successfully completed in bug 1007615, so closing this as duplicate.

*** This bug has been marked as a duplicate of bug 1007615 ***


Note You need to log in before you can comment on or make changes to this bug.