| Summary: | Review Request: endless-sky - Space exploration, trading, and combat game | ||
|---|---|---|---|
| Product: | [Fedora] Fedora | Reporter: | Link Dupont <link> |
| Component: | Package Review | Assignee: | Zbigniew Jędrzejewski-Szmek <zbyszek> |
| Status: | CLOSED ERRATA | QA Contact: | Fedora Extras Quality Assurance <extras-qa> |
| Severity: | medium | Docs Contact: | |
| Priority: | unspecified | ||
| Version: | rawhide | CC: | package-review, zbyszek |
| Target Milestone: | --- | Flags: | zbyszek:
fedora-review+
|
| Target Release: | --- | ||
| Hardware: | All | ||
| OS: | Linux | ||
| Whiteboard: | |||
| Fixed In Version: | Doc Type: | Bug Fix | |
| Doc Text: | Story Points: | --- | |
| Clone Of: | Environment: | ||
| Last Closed: | 2016-11-19 21:08:03 UTC | Type: | --- |
| Regression: | --- | Mount Type: | --- |
| Documentation: | --- | CRM: | |
| Verified Versions: | Category: | --- | |
| oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |
| Cloudforms Team: | --- | Target Upstream Version: | |
|
Description
Link Dupont
2016-01-11 05:43:43 UTC
linkdupont's scratch build of endless-sky-0.8.10-3.fc23.src.rpm for f23 failed http://koji.fedoraproject.org/koji/taskinfo?taskID=12494690 linkdupont's scratch build of endless-sky-0.8.10-3.fc23.src.rpm for f23 completed http://koji.fedoraproject.org/koji/taskinfo?taskID=12494828 Updated SPEC file with an enhanced patch that fixes the game at runtime when looking for data files. Spec URL: https://fedorapeople.org/cgit/linkdupont/public_git/packages.git/tree/SPECS/endless-sky.spec SRPM URL: https://linkdupont.fedorapeople.org/srpms/endless-sky-0.8.10-4.fc23.src.rpm koji build for 0.8.10-4: http://koji.fedoraproject.org/koji/taskinfo?taskID=12509931 Oh, a game, nice. Even has appdata, even nicer.
In the future, please link to plain text version of the spec file. HTML breaks fedora-review and other automated tools.
Requires: %{name}-data → Requires: %{name}-data = %{version}-%{release}
You don't want to deal with bug reports from people who upgrade the main package but not the data package, or the other way around. It's best to ensure that they are always in lockstep.
You have mixed licensing. You should document the licensing in a comment in the spec file.
"cp %{SOURCE1} ." can be replaced with "-a1" argument to %autosetup.
Empty %doc in %files data: I don't think this does anything, can be removed.
fedora-review says:
- gtk-update-icon-cache is invoked in %postun and %posttrans if package
contains icons.
Note: icons in endless-sky
See: http://fedoraproject.org/wiki/Packaging:ScriptletSnippets#Icon_Cache
- Package installs a %{name}.desktop using desktop-file-install or desktop-
file-validate if there is such a file.
You should also call appstream-util validate-relax --nonet on the appstream file in %check. You should also manually call appstream-util validate by hand, and fix the issues it reports. This is more strict, so you don't want to use it in %check:
/usr/share/appdata/endless-sky.appdata.xml: FAILED:
• tag-missing : <update_contact> is not present
• style-invalid : <caption> is too long [Finding trade routes, made easy: the map view shows commodity prices for other star systems and can color them based on price, to help you decide what trade goods you should buy in the current system for sale elsewhere]
• style-invalid : <caption> is too long [Strange discoveries await you beyond the boundaries of known space]
• style-invalid : <caption> is too long [You can earn a living just by buying commodities for a low price in one star system and selling them at a profit elsewhere]
• style-invalid : <developer_name> is too long [XXX: Insert Company or Developer Name]
Validation of files failed
I'd be happy to sponsor you into the packagers group. Please do two or three reviews of packages from http://fedoraproject.org/PackageReviewStatus/NEW.html. Running fedora-review is a good first step, but please note that the automatically generated template needs to be filled in in various places, and trimmed in others. Not everything the tools say is always correct. Sometimes they are outdated, sometimes they are plain wrong. It's always best to link to the relevant part of the guidelines. Please pick packages that are in the area you are interested in, so that you can finalize the review after you get the packager bit. If you have any questions or issues, I'll try to help (zbyszek at in waw pl, zbyszek on #fedora-devel).
(In reply to Zbigniew Jędrzejewski-Szmek from comment #4) > Oh, a game, nice. Even has appdata, even nicer. > > In the future, please link to plain text version of the spec file. HTML > breaks fedora-review and other automated tools. > > Requires: %{name}-data → Requires: %{name}-data = %{version}-%{release} > You don't want to deal with bug reports from people who upgrade the main > package but not the data package, or the other way around. It's best to > ensure that they are always in lockstep. Done. > You have mixed licensing. You should document the licensing in a comment in > the spec file. Done. > "cp %{SOURCE1} ." can be replaced with "-a1" argument to %autosetup. The -a# tries to uncompress the source. In this case, my %SOURCE1 is not a compressed file. It's just XML. I could compress it so that "-a1" works, but that felt like more work on my part to maintain changes to the appdata.xml. > Empty %doc in %files data: I don't think this does anything, can be removed. Done. > fedora-review says: > - gtk-update-icon-cache is invoked in %postun and %posttrans if package > contains icons. > Note: icons in endless-sky > See: http://fedoraproject.org/wiki/Packaging:ScriptletSnippets#Icon_Cache Added the scriptlets. > - Package installs a %{name}.desktop using desktop-file-install or desktop- > file-validate if there is such a file. Was there a change that needed to be made from this recommendation? Does the SConstruct not install via desktop-file-install? > You should also call appstream-util validate-relax --nonet on the appstream > file in %check. You should also manually call appstream-util validate by > hand, and fix the issues it reports. This is more strict, so you don't want > to use it in %check: > > /usr/share/appdata/endless-sky.appdata.xml: FAILED: > • tag-missing : <update_contact> is not present > • style-invalid : <caption> is too long [Finding trade routes, made > easy: the map view shows commodity prices for other star systems and can > color them based on price, to help you decide what trade goods you should > buy in the current system for sale elsewhere] > • style-invalid : <caption> is too long [Strange discoveries await > you beyond the boundaries of known space] > • style-invalid : <caption> is too long [You can earn a living just > by buying commodities for a low price in one star system and selling them at > a profit elsewhere] > • style-invalid : <developer_name> is too long [XXX: Insert Company > or Developer Name] > Validation of files failed > Fixed all that up and got some better screenshots. > > I'd be happy to sponsor you into the packagers group. Please do two or three > reviews of packages from > http://fedoraproject.org/PackageReviewStatus/NEW.html. Running fedora-review > is a good first step, but please note that the automatically generated > template needs to be filled in in various places, and trimmed in others. Not > everything the tools say is always correct. Sometimes they are outdated, > sometimes they are plain wrong. It's always best to link to the relevant > part of the guidelines. Please pick packages that are in the area you are > interested in, so that you can finalize the review after you get the > packager bit. If you have any questions or issues, I'll try to help (zbyszek > at in waw pl, zbyszek on #fedora-devel). Thank you! -- http://copr.fedoraproject.org/coprs/linkdupont/fedora-link-extras/build/154038/ copr build failed when building in rawhide. > + appstream-util validate-relax --nonet /builddir/build/SOURCES/endless-sky.appdata.xml > /builddir/build/SOURCES/endless-sky.appdata.xml: GLib-GIO-Message: Using the 'memory' GSettings backend. Your settings will not be saved or shared with other applications. > FAILED: > ? tag-invalid : <project_group> is not valid [none] > Validation of files failed Has the appdata spec changed in rawhide? linkdupont's scratch build of endless-sky-0.8.10-5.fc23.src.rpm for f23 completed http://koji.fedoraproject.org/koji/taskinfo?taskID=12587946 (In reply to Link Dupont from comment #5) > (In reply to Zbigniew Jędrzejewski-Szmek from comment #4) > > - Package installs a %{name}.desktop using desktop-file-install or desktop- > > file-validate if there is such a file. > > Was there a change that needed to be made from this recommendation? Does the > SConstruct not install via desktop-file-install? I added a call to desktop-file-validate in %check > http://copr.fedoraproject.org/coprs/linkdupont/fedora-link-extras/build/ > 154038/ > > > copr build failed when building in rawhide. > > > + appstream-util validate-relax --nonet /builddir/build/SOURCES/endless-sky.appdata.xml > > /builddir/build/SOURCES/endless-sky.appdata.xml: GLib-GIO-Message: Using the 'memory' GSettings backend. Your settings will not be saved or shared with other applications. > > FAILED: > > ? tag-invalid : <project_group> is not valid [none] > > Validation of files failed > > Has the appdata spec changed in rawhide? Looks like <project_group> is optional, so I removed it from the appdata.xml. Latest koji build: http://koji.fedoraproject.org/koji/taskinfo?taskID=12588229 Updated SPEC: https://fedorapeople.org/cgit/linkdupont/public_git/packages.git/plain/SPECS/endless-sky.spec Updated SRPM: https://linkdupont.fedorapeople.org/srpms/endless-sky-0.8.10-5.fc23.src.rpm (In reply to Link Dupont from comment #5) > (In reply to Zbigniew Jędrzejewski-Szmek from comment #4) > > "cp %{SOURCE1} ." can be replaced with "-a1" argument to %autosetup. > > The -a# tries to uncompress the source. In this case, my %SOURCE1 is not a > compressed file. It's just XML. I could compress it so that "-a1" works, but > that felt like more work on my part to maintain changes to the appdata.xml. I didn't know that. cp is fine of course. > > - Package installs a %{name}.desktop using desktop-file-install or desktop- > > file-validate if there is such a file. > > Was there a change that needed to be made from this recommendation? Does the > SConstruct not install via desktop-file-install? OK, I missed that, so it's not necessary ... > I added a call to desktop-file-validate in %check ... but doing it explicitly is probably better. > > You should also call appstream-util validate-relax --nonet on the appstream > > file in %check. You should also manually call appstream-util validate by > > hand, and fix the issues it reports. This is more strict, so you don't want > > to use it in %check: > > > > /usr/share/appdata/endless-sky.appdata.xml: FAILED: > > • tag-missing : <update_contact> is not present > > • style-invalid : <caption> is too long [Finding trade routes, made > > easy: the map view shows commodity prices for other star systems and can > > color them based on price, to help you decide what trade goods you should > > buy in the current system for sale elsewhere] > > • style-invalid : <caption> is too long [Strange discoveries await > > you beyond the boundaries of known space] > > • style-invalid : <caption> is too long [You can earn a living just > > by buying commodities for a low price in one star system and selling them at > > a profit elsewhere] > > • style-invalid : <developer_name> is too long [XXX: Insert Company > > or Developer Name] > > Validation of files failed > > > > Fixed all that up and got some better screenshots. ... > Has the appdata spec changed in rawhide? Maybe, it changes all the time ;) The litmus test is whether gnome-software shows it. On my F23 machine it didn't want to show the previous version, but it shows this one, and it looks great. + latest version + license is acceptable for Fedora + license files are present, %license is used + latest version + builds, installs, runs OK + has appdata and desktop files + scriptlets look sane + provides and requires are OK Package is APPROVED. Hey, any news here? Hi, sorry to have dropped the ball on this. Life caught up with me real quick and it doesn't look like I'll have the time to commit to this right now. Anyone else is free to pick up where I left off. would be nice to get this. The package and the software are good. Link is busy, but somebody could take over and resubmit the package. Link can become comaintainer later. I can take this now. So... package is good, but you need packager privileges. See the bottom part of comment #c4 for the next steps. Also, please fill in your full name in FAS (https://admin.fedoraproject.org/accounts/user/view/linkdupont). (In reply to Zbigniew Jędrzejewski-Szmek from comment #14) > So... package is good, but you need packager privileges. See the bottom part > of comment #c4 for the next steps. Also, please fill in your full name in > FAS (https://admin.fedoraproject.org/accounts/user/view/linkdupont). Oh yea. I did a review of flyingsaucersattack on 1303349. Looks like there's a follow-up for me. I'll track down a couple others. I do have my full name in FAS. Is it not showing up? (In reply to Link Dupont from comment #15) > Oh yea. I did a review of flyingsaucersattack on 1303349. Looks like there's > a follow-up for me. I'll track down a couple others. OK, great. > I do have my full name in FAS. Is it not showing up? Doesn't seem to be. There's no "Full name" entry, only "Account name" and "Email" fields. zodbot also doesn't show anything: 16:14 <zodbot> User: linkdupont, Name: None, email: link, Creation: 2015-01-01, IRC Nick: None, Timezone: None, Locale: None, GPG key ID: None, Status: active 16:14 <zodbot> Approved Groups: fedorabugs cla_done cla_fpca Oh, I bet it was because I had "Privacy" checked. I unchecked it. I also reviewed libtecla on 1358917. Yep, I can see the FAS data now. Your reviews in 1358917 and 1303349 are very good. I've added you to the packagers group. Have fun and use your powers for good. If you have any questions, I'll be happy to provide assistance if possible (just use my bugzilla e-mail). https://admin.fedoraproject.org/pkgdb/request/package/ is the next step. Package request has been approved: https://admin.fedoraproject.org/pkgdb/package/rpms/endless-sky endless-sky-0.9.4-1.fc23 has been submitted as an update to Fedora 23. https://bodhi.fedoraproject.org/updates/FEDORA-2016-3f9fed873a endless-sky-0.9.4-1.fc24 has been submitted as an update to Fedora 24. https://bodhi.fedoraproject.org/updates/FEDORA-2016-325897ba92 endless-sky-0.9.4-1.fc25 has been submitted as an update to Fedora 25. https://bodhi.fedoraproject.org/updates/FEDORA-2016-51b9047635 endless-sky-0.9.4-1.fc25 has been pushed to the Fedora 25 testing repository. If problems still persist, please make note of it in this bug report. See https://fedoraproject.org/wiki/QA:Updates_Testing for instructions on how to install test updates. You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2016-51b9047635 endless-sky-0.9.4-1.fc24 has been pushed to the Fedora 24 testing repository. If problems still persist, please make note of it in this bug report. See https://fedoraproject.org/wiki/QA:Updates_Testing for instructions on how to install test updates. You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2016-325897ba92 endless-sky-0.9.4-1.fc23 has been submitted as an update to Fedora 23. https://bodhi.fedoraproject.org/updates/FEDORA-2016-3f9fed873a endless-sky-0.9.4-1.fc23 has been pushed to the Fedora 23 testing repository. If problems still persist, please make note of it in this bug report. See https://fedoraproject.org/wiki/QA:Updates_Testing for instructions on how to install test updates. You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2016-3f9fed873a endless-sky-0.9.4-1.fc25 has been pushed to the Fedora 25 stable repository. If problems still persist, please make note of it in this bug report. endless-sky-0.9.4-1.fc23 has been pushed to the Fedora 23 stable repository. If problems still persist, please make note of it in this bug report. endless-sky-0.9.4-1.fc24 has been pushed to the Fedora 24 stable repository. If problems still persist, please make note of it in this bug report. |