Bug 1362130 - Review Request: olpcau-abc123-fonts - A nice font for kids/readability
Summary: Review Request: olpcau-abc123-fonts - A nice font for kids/readability
Keywords:
Status: CLOSED INSUFFICIENT_DATA
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
unspecified
medium
Target Milestone: ---
Assignee: Zbigniew Jędrzejewski-Szmek
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2016-08-01 10:49 UTC by Sam P.
Modified: 2019-09-17 15:37 UTC (History)
3 users (show)

Fixed In Version:
Doc Type: If docs needed, set a value
Doc Text:
Clone Of:
Environment:
Last Closed: 2019-09-17 15:37:10 UTC
Type: ---
Embargoed:
zbyszek: fedora-review?


Attachments (Terms of Use)

Description Sam P. 2016-08-01 10:49:44 UTC
Spec URL:  http://people.sugarlabs.org/sam/olpcau-abc123-fonts.spec
SRPM URL:  https://copr-be.cloud.fedoraproject.org/results/samtoday/olpcau-abc123-fonts/fedora-24-x86_64/00433182-olpcau-abc123-fonts/olpcau-abc123-fonts-20130716-1.fc24.src.rpm
COPR:  https://copr.fedorainfracloud.org/coprs/samtoday/olpcau-abc123-fonts/
Description:

abc123 is a typeface developed for literacy John Greatorex, Sridhar
Dhanapalan and Ruben Rodriguez for One Laptop per Child Australia.
abc123 includes letters in the same shapes that children are taught to
write them and avoids shapes hard to read for small children.

Fedora Account System Username:  samtoday

Comment 1 Zbigniew Jędrzejewski-Szmek 2016-08-12 14:43:13 UTC
The Spec URL doesn't seem to work for me, I'm using http://copr-dist-git.fedorainfracloud.org/cgit/samtoday/olpcau-abc123-fonts/olpcau-abc123-fonts.git/commit/?h=f24

Summary should not repeat the package name. It also doesn't need the leading article (it looks better in listing if there's no article, since otherwise almost everything would start with "A").

Group: User Interface/X
→ not used [https://fedoraproject.org/wiki/Packaging:Guidelines#Tags_and_Sections]

I don't think you have to copy sources in %prep. This breaks 'fedpkg local' builds too.

appstream-util validate-relax --nonet should be added in %check:
https://fedoraproject.org/wiki/Packaging:AppData#app-data-validate_usage.

You should also run full validation locally with "validate" and fix issues:
olpcau-abc123.metainfo.xml: FAILED:
• style-invalid         : <summary> requires sentence case [abc123, a typeface developed for literacy]
Validation of files failed

Package looks good.

--

I can sponsor you into the packager group. Apart from fixing the few issues pointed out above, I would like you to do two or three reviews of other packages (https://fedoraproject.org/PackageReviewStatus/NEW.html is a good start) and to be familiar with mock, fedora-review, etc. There's a bunch of fonts packages waiting for review, so you might want to take those (or not, whatever you prefer). Until you're a packager, you cannot formally approve a package, so please don't assign the review to yourself, just paste whatever comments you have. If nobody beats you to it, you will be able to formally approve those packages after you become packager. If you have any questions, feel free to mail me, or ping on IRC (I'm "zbyszek" everywhere).

Comment 2 Parag AN(पराग) 2016-08-13 05:42:10 UTC
Zbigniew,

Just my thought here that removal of 177841 at this stage looks a proactive action here. Generally we (Sponsor) make sure that package submitter is having a understanding of rpm packaging by asking him/her to review others packages waiting for their reviews and complete this package review and then remove the 177841 blocker.

Comment 3 Zbigniew Jędrzejewski-Szmek 2016-08-13 16:57:22 UTC
I'm not sure ;) I unset the flag when I make the offer to sponsor someone because I assume that other sponsors are looking at bugs with that flag for people to sponsor. Please correct me if that's not the case.

Comment 4 Parag AN(पराग) 2016-08-14 17:15:13 UTC
I am using this as general practice that let the review be on completion along with sponsorship process requirements and then remove 177841 and sponsor. 

If you are ready to sponsor then first step is to assign the review to yourself and change the flag to fedora-review? This will make sure other sponsors that you already picked this for review and then will be knowing you will complete the sponsorship.

Comment 5 Zbigniew Jędrzejewski-Szmek 2016-11-12 15:35:12 UTC
Any progress?

Comment 6 Sam P. 2016-11-14 11:44:29 UTC
Thanks for the review Zbigniew!

> I don't think you have to copy sources in %prep. This breaks 'fedpkg local' builds too.

I tried removing the copy, and it continued to work via mock and rpmbuild locally on my computer.  However, it caused it to fail when building on COPR [1].  So I kept it in the end.

I've updated the package:

Spec URL:  http://copr-dist-git.fedorainfracloud.org/cgit/samtoday/olpcau-abc123-fonts/olpcau-abc123-fonts.git/plain/olpcau-abc123-fonts.spec?id=68ad2759b09e46cd16f1ac7e3935c6bcce1766a7  
SRPM URL:  https://copr-be.cloud.fedoraproject.org/results/samtoday/olpcau-abc123-fonts/fedora-25-x86_64/00476460-olpcau-abc123-fonts/olpcau-abc123-fonts-20130716-2.fc25.src.rpm

[1] https://copr.fedorainfracloud.org/coprs/samtoday/olpcau-abc123-fonts/build/476456/ and https://copr-be.cloud.fedoraproject.org/results/samtoday/olpcau-abc123-fonts/fedora-25-x86_64/00476456-olpcau-abc123-fonts/build.log.gz

Comment 7 Zbigniew Jędrzejewski-Szmek 2016-11-14 18:01:58 UTC
developed for literacy John Greatorex → missing "by"

The package looks good. I'll approve the package once you are sponsored into the packagers group. Any chance you could review some other packages?

Comment 9 Zbigniew Jędrzejewski-Szmek 2017-03-29 02:37:29 UTC
Hi, sorry for the long silence. I went away on vacation and then completely forgot about this. For the future, in such cases, it often helps to send a ping after a week or two, just to get things going again. Let's get this ball rolling again!

I looked at your comments on those reviews, but they are a bit scant. Before I asked you for a fedora-review run on #1388945. I'd like to know that you have set up mock and can do test builds there. Can you please do a review of one more package, this time using fedora-review as the starting point?

Comment 10 Zbigniew Jędrzejewski-Szmek 2019-09-17 15:37:10 UTC
Let's close this. Feel free to restart the process at any time.


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