Bug 1583798 (termy-qt)

The stuff inside /usr/share/qtermy is 6.2 MiB. It should go in termy-qt-data or some other noarch subpackage, IMHO.

Sounds reasonable. Will plan on doing this pending further review.

The libv8 package in Fedora is now deprecated [1]. To accommodate this, I'm working on bundling libv8 upstream and doing a static link instead. Because of this, the size of the termy-qt executable will increase. There will be a net savings in space over the current package+libv8. However, the executable is going to be larger than all of the data in /usr/share/qtermy combined, so I don't believe that a subpackage will be necessary.

[1] https://lists.fedoraproject.org/archives/list/devel@lists.fedoraproject.org/message/DI4Q5526MVI5KS7OG4PH37QFK6KCDAY2/

I'd prefer if:
Source:  https://termysequence.io/releases/termysequence-%{version}.tar.xz
Source1: https://termysequence.io/releases/termy-icon-theme-%{icons_version}.tar.xz
Source2: https://termysequence.io/releases/termy-emoji-%{emoji_version}.tar.xz

These 3 items were packaged separately.  I assume you're bundling them all here for convenience?

Okay, a new 1.1.0 release has been made upstream. Upstream is now distributing termy-qt as a separate tarball. Upstream is also no longer distributing termy-emoji and termy-icon-theme tarballs. These are now bundled into termy-qt. The V8 library is also now bundled as described in comment #3.

Spec URL: https://termysequence.io/fedora/termy-qt.spec
SRPM URL: https://termysequence.io/fedora/termy-qt-1.1.0-1.fc28.src.rpm

Available in COPR:
https://copr.fedorainfracloud.org/coprs/ewalsh/termysequence/

$ rpmlint *.src.rpm
1 packages and 0 specfiles checked; 0 errors, 0 warnings.

naming: ok

licensing: ok
but consider adding a comment near the License: tag as reference to which licenses apply to which parts of the software

sources: ok
$ md5sum *.xz
ac7581529766edb8f8c89f4515c94a15  termysequence-qt-1.1.0.tar.xz

1.  similar to termy-sever review, SHOULD remove
# Build type "None" disables Release/Debug CFLAGS and LDFLAGS set by CMake.
# Only the CFLAGS and LDFLAGS specified by rpmbuild will be used.
and consider using
-DCMAKE_BUILD_TYPE=Release
instead.

scriptlets: n/a

macros: ok

2.  since this bundles a copy of v8, MUST add something like:
Provides: bundled(v8) = <version>
per 
https://fedoraproject.org/wiki/Bundled_Libraries?rd=Packaging:Bundled_Libraries#Requirement_if_you_bundle

I have made all of the changes described in comment #6 and corrected the License which was missing BSD for v8. I have also updated to the upstream 1.1.1 release.

Spec URL: https://termysequence.io/fedora/termy-qt.spec
SRPM URL: https://termysequence.io/fedora/termy-qt-1.1.1-1.fc28.src.rpm

Available in COPR:
https://copr.fedorainfracloud.org/coprs/ewalsh/termysequence/

Koji scratch builds:
f28 https://koji.fedoraproject.org/koji/taskinfo?taskID=29790338
f29 https://koji.fedoraproject.org/koji/taskinfo?taskID=29790342
f30 https://koji.fedoraproject.org/koji/taskinfo?taskID=29790346

Thanks looks good, approved.


Minor things to consider in the future, 

1. avoid using globs like:

desktop-file-validate %{buildroot}%{_datadir}/applications/*.desktop
appstream-util validate-relax --nonet %{buildroot}%{_datadir}/metainfo/*.appdata.xml

and 

%{_datadir}/applications/*.desktop
%{_datadir}/metainfo/*.appdata.xml

and reference the full files precisely (without globs).  These should generally never change, and if they do, you should be mindful of that.

2. consider using
%{_metainfodir}/...
instead of hard-coded
 %{_datadir}/metainfo/...

(fedscm-admin):  The Pagure repository was created at https://src.fedoraproject.org/rpms/termy-qt

termy-qt-1.1.1-1.fc29 has been submitted as an update to Fedora 29. https://bodhi.fedoraproject.org/updates/FEDORA-2018-41064bd1fa

termy-qt-1.1.1-1.fc28 has been submitted as an update to Fedora 28. https://bodhi.fedoraproject.org/updates/FEDORA-2018-e6fc2fe6c7

termy-qt-1.1.1-1.fc29 has been pushed to the Fedora 29 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-2018-41064bd1fa

termy-qt-1.1.1-1.fc28 has been pushed to the Fedora 28 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-2018-e6fc2fe6c7

I have committed the changes described in comment #8 and they will be present in the next release. Thanks again for the sponsorship and the review. Feel free to give me a review or two to work on in return.

termy-qt-1.1.1-1.fc28 has been pushed to the Fedora 28 stable repository. If problems still persist, please make note of it in this bug report.

termy-qt-1.1.1-1.fc29 has been pushed to the Fedora 29 stable repository. If problems still persist, please make note of it in this bug report.