Note: This is a public test instance of Red Hat Bugzilla. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback at bugzilla.redhat.com.
Bug 1583798 (termy-qt)
Summary: | Review Request: termy-qt - TermySequence terminal multiplexer client | ||
---|---|---|---|
Product: | [Fedora] Fedora | Reporter: | Eamon Walsh <ewalsh> |
Component: | Package Review | Assignee: | Rex Dieter <rdieter> |
Status: | CLOSED ERRATA | QA Contact: | Fedora Extras Quality Assurance <extras-qa> |
Severity: | medium | Docs Contact: | |
Priority: | unspecified | ||
Version: | rawhide | CC: | fedora, package-review, rdieter |
Target Milestone: | --- | Flags: | rdieter:
fedora-review+
|
Target Release: | --- | ||
Hardware: | All | ||
OS: | Linux | ||
Whiteboard: | |||
Fixed In Version: | Doc Type: | If docs needed, set a value | |
Doc Text: | Story Points: | --- | |
Clone Of: | Environment: | ||
Last Closed: | 2018-10-01 01:08:37 UTC | Type: | --- |
Regression: | --- | Mount Type: | --- |
Documentation: | --- | CRM: | |
Verified Versions: | Category: | --- | |
oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |
Cloudforms Team: | --- | Target Upstream Version: | |
Embargoed: | |||
Bug Depends On: | 1582983 | ||
Bug Blocks: | 928937 |
Description
Eamon Walsh
2018-05-29 17:46:53 UTC
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. |