Bug 1310607
Summary: | Review Request: qutebrowser - A keyboard-driven, vim-like browser based on PyQt5 and QtWebKit | ||
---|---|---|---|
Product: | [Fedora] Fedora | Reporter: | Tomas Orsava <torsava> |
Component: | Package Review | Assignee: | Bohuslav "Slavek" Kabrda <bkabrda> |
Status: | CLOSED NEXTRELEASE | QA Contact: | Fedora Extras Quality Assurance <extras-qa> |
Severity: | medium | Docs Contact: | |
Priority: | medium | ||
Version: | rawhide | CC: | bkabrda, fedoraproject.org, package-review |
Target Milestone: | --- | Flags: | bkabrda:
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-02-23 10:28:55 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: |
Description
Tomas Orsava
2016-02-22 10:51:24 UTC
Taking this for review. - All the dependencies are used in python3 version, but cssutils is python2 - is there any reason for that? IMO it's a typo. - There are quite a lot hashbangs that point to /usr/bin/env. While there is nothing in guidelines (that I know of) that would prohibit this, I'd advise changing these to point to /usr/bin/python3. Consider this - someone might try to run this in an environment where "/usr/bin/env python3" points to something else than /usr/bin/python3. qutebrowser would likely fail since it wouldn't find dependencies (dependencies that it has been built with and tested with by you as a packager). By replacing the hashbangs, qutebrowser can keep working even in environments like this (well, for the most part, anyway). Otherwise everything looks good - the package builds and runs fine, rpmlint output is clean, licensing is good and specfile looks very nice. Once the above problems are solved, this package can be approved (oh, and I like that it's Python 3!) (In reply to Bohuslav "Slavek" Kabrda from comment #2) > - All the dependencies are used in python3 version, but cssutils is python2 > - is there any reason for that? IMO it's a typo. Since cssutils is only an optional dependency, I have commented that line out to be readded in the future when cssutils is packaged for Python 3. > - There are quite a lot hashbangs that point to /usr/bin/env. While there is > nothing in guidelines (that I know of) that would prohibit this, I'd advise > changing these to point to /usr/bin/python3. Consider this - someone might > try to run this in an environment where "/usr/bin/env python3" points to > something else than /usr/bin/python3. qutebrowser would likely fail since it > wouldn't find dependencies (dependencies that it has been built with and > tested with by you as a packager). By replacing the hashbangs, qutebrowser > can keep working even in environments like this (well, for the most part, > anyway). I have added a line into the %build section that changes those hashbangs accordingly. > > Otherwise everything looks good - the package builds and runs fine, rpmlint > output is clean, licensing is good and specfile looks very nice. Once the > above problems are solved, this package can be approved (oh, and I like that > it's Python 3!) Thanks for all your input, hopefully now it'll be all ok! Spec URL: https://torsava.fedorapeople.org/qutebrowser.spec SRPM URL: https://torsava.fedorapeople.org/qutebrowser-0.5.1-1.fc23.src.rpm LGTM, APPROVED. Note for future reviews, that it is customary to include a changelog entry and mention your changes during review there (for every review round, if there are more). Let's leave the specfile as it is for this instance. Ah, sorry, next time I'll make it so. Thanks! Package request has been approved: https://admin.fedoraproject.org/pkgdb/package/qutebrowser |