|Summary:||Review Request: olpcau-abc123-fonts - A nice font for kids/readability|
|Product:||[Fedora] Fedora||Reporter:||Sam P. <sam>|
|Component:||Package Review||Assignee:||Zbigniew Jędrzejewski-Szmek <zbyszek>|
|Status:||ASSIGNED ---||QA Contact:||Fedora Extras Quality Assurance <extras-qa>|
|Version:||rawhide||CC:||package-review, sam, zbyszek|
|Fixed In Version:||Doc Type:||If docs needed, set a value|
|Doc Text:||Story Points:||---|
|oVirt Team:||---||RHEL 7.3 requirements from Atomic Host:|
|Cloudforms Team:||---||Target Upstream Version:|
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
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 . 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  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 8 Sam P. 2016-11-15 09:37:39 UTC
Thanks, I have updated the package to fix the typo: Spec URL: http://copr-dist-git.fedorainfracloud.org/cgit/samtoday/olpcau-abc123-fonts/olpcau-abc123-fonts.git/plain/olpcau-abc123-fonts.spec?id=307918b5e8beba9bf988a93badfcc34ea0fedeec SRPM URL: https://copr-be.cloud.fedoraproject.org/results/samtoday/olpcau-abc123-fonts/fedora-25-x86_64/00476808-olpcau-abc123-fonts/olpcau-abc123-fonts-20130716-3.fc25.src.rpm I also found 2 packages that I have reviewed: * https://bugzilla.redhat.com/show_bug.cgi?id=1388945 * https://bugzilla.redhat.com/show_bug.cgi?id=1377631
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?