Bug 1297281 - Review Request: endless-sky - Space exploration, trading, and combat game
Summary: Review Request: endless-sky - Space exploration, trading, and combat game
Keywords:
Status: CLOSED ERRATA
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-01-11 05:43 UTC by Link Dupont
Modified: 2016-11-30 03:51 UTC (History)
2 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2016-11-19 21:08:03 UTC
Type: ---
Embargoed:
zbyszek: fedora-review+


Attachments (Terms of Use)

Description Link Dupont 2016-01-11 05:43:43 UTC
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-2.fc23.src.rpm
Description: Explore other star systems. Earn money by trading, carrying passengers, or
completing missions. Use your earnings to buy a better ship or to upgrade the
weapons and engines on your current one. Blow up pirates. Take sides in a civil
war. Or leave human space behind and hope to find some friendly aliens whose
culture is more civilized than your own...
Fedora Account System Username: linkdupont

This is my first package, so I will need a sponsor.

Koji scratch build for f24: http://koji.fedoraproject.org/koji/taskinfo?taskID=12494665
Copr build: http://copr.fedoraproject.org/coprs/linkdupont/fedora-link-extras/build/152639/

Comment 1 Upstream Release Monitoring 2016-01-11 06:19:13 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

Comment 2 Upstream Release Monitoring 2016-01-11 07:21:07 UTC
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

Comment 3 Link Dupont 2016-01-12 05:44:15 UTC
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

Comment 4 Zbigniew Jędrzejewski-Szmek 2016-01-17 01:15:14 UTC
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).

Comment 5 Link Dupont 2016-01-17 20:43:40 UTC
(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?

Comment 6 Upstream Release Monitoring 2016-01-17 20:56:20 UTC
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

Comment 7 Link Dupont 2016-01-17 22:23:02 UTC
(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

Comment 8 Zbigniew Jędrzejewski-Szmek 2016-01-18 16:46:43 UTC
(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.

Comment 9 Zbigniew Jędrzejewski-Szmek 2016-02-20 22:31:39 UTC
Hey, any news here?

Comment 10 Link Dupont 2016-07-07 05:14:45 UTC
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.

Comment 11 Igor Gnatenko 2016-08-14 15:39:59 UTC
would be nice to get this.

Comment 12 Zbigniew Jędrzejewski-Szmek 2016-08-14 18:54:25 UTC
The package and the software are good. Link is busy, but somebody could take over and resubmit the package. Link can become comaintainer later.

Comment 13 Link Dupont 2016-10-15 06:39:59 UTC
I can take this now.

Comment 14 Zbigniew Jędrzejewski-Szmek 2016-10-15 15:58:44 UTC
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).

Comment 15 Link Dupont 2016-10-16 05:15:27 UTC
(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?

Comment 16 Zbigniew Jędrzejewski-Szmek 2016-10-16 16:16:02 UTC
(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

Comment 17 Link Dupont 2016-10-19 04:49:33 UTC
Oh, I bet it was because I had "Privacy" checked. I unchecked it.

Comment 18 Link Dupont 2016-10-19 05:33:37 UTC
I also reviewed libtecla on 1358917.

Comment 19 Zbigniew Jędrzejewski-Szmek 2016-10-29 18:18:26 UTC
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).

Comment 20 Zbigniew Jędrzejewski-Szmek 2016-10-29 18:20:27 UTC
https://admin.fedoraproject.org/pkgdb/request/package/ is the next step.

Comment 21 Gwyn Ciesla 2016-10-31 12:55:47 UTC
Package request has been approved: https://admin.fedoraproject.org/pkgdb/package/rpms/endless-sky

Comment 22 Fedora Update System 2016-11-02 05:21:15 UTC
endless-sky-0.9.4-1.fc23 has been submitted as an update to Fedora 23. https://bodhi.fedoraproject.org/updates/FEDORA-2016-3f9fed873a

Comment 23 Fedora Update System 2016-11-02 05:21:22 UTC
endless-sky-0.9.4-1.fc24 has been submitted as an update to Fedora 24. https://bodhi.fedoraproject.org/updates/FEDORA-2016-325897ba92

Comment 24 Fedora Update System 2016-11-02 05:21:27 UTC
endless-sky-0.9.4-1.fc25 has been submitted as an update to Fedora 25. https://bodhi.fedoraproject.org/updates/FEDORA-2016-51b9047635

Comment 25 Fedora Update System 2016-11-02 14:53:05 UTC
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

Comment 26 Fedora Update System 2016-11-05 03:32:08 UTC
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

Comment 27 Fedora Update System 2016-11-05 03:48:20 UTC
endless-sky-0.9.4-1.fc23 has been submitted as an update to Fedora 23. https://bodhi.fedoraproject.org/updates/FEDORA-2016-3f9fed873a

Comment 28 Fedora Update System 2016-11-06 01:52:03 UTC
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

Comment 29 Fedora Update System 2016-11-19 21:08:03 UTC
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.

Comment 30 Fedora Update System 2016-11-29 23:52:20 UTC
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.

Comment 31 Fedora Update System 2016-11-30 03:51:27 UTC
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.


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