Bug 564537
| Summary: | Review Request: grc - simple python logfile colouriser | |||
|---|---|---|---|---|
| Product: | [Fedora] Fedora | Reporter: | Carl van Tonder <carl> | |
| Component: | Package Review | Assignee: | Buland Kumar Singh <i.am.fedora.bk> | |
| Status: | CLOSED NOTABUG | QA Contact: | Fedora Extras Quality Assurance <extras-qa> | |
| Severity: | medium | Docs Contact: | ||
| Priority: | low | |||
| Version: | rawhide | CC: | fedora-package-review, i.am.fedora.bk, i, pahan, terje.rosten | |
| Target Milestone: | --- | Keywords: | Reopened | |
| Target Release: | --- | Flags: | i.am.fedora.bk:
fedora-review?
|
|
| Hardware: | All | |||
| OS: | Linux | |||
| Whiteboard: | ||||
| Fixed In Version: | Doc Type: | Bug Fix | ||
| Doc Text: | Story Points: | --- | ||
| Clone Of: | ||||
| : | 1200038 (view as bug list) | Environment: | ||
| Last Closed: | 2014-07-12 13:24:32 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
Carl van Tonder
2010-02-13 03:32:21 UTC
Some comments:
Release: 0.1%{?dist}.svac
Why so uncommon release tag, what't wrong with 1%{?dist}?
%build
- Add a comment about why %build in empty.
%install
rm -rf $RPM_BUILD_ROOT
%{_builddir}/grc-1.3/install.sh %{_builddir}/grc-1.3/ $RPM_BUILD_ROOT
- %install will cd into source dir, you should use:
./install.sh $RPM_BUILD_ROOT
I don't understand why you need the build patch, (which is a install patch:-)
Spec URL: http://supervacuo.com/fedora/grc.spec SRPM URL: http://supervacuo.com/fedora/grc-1.3-0.2.fc12.src.rpm > Why so uncommon release tag, what't wrong with 1%{?dist}? It started life as a "personal use" package; fixed > Add a comment about why %build in empty. Done; it's because there is no "compilation" step — grc and grcat are python scripts > %install will cd into source dir, you should use: > ./install.sh $RPM_BUILD_ROOT > I don't understand why you need the build patch, (which is a install patch:-) For some reason, specifying relative paths seemed not to work yesterday, but now it does indeed work. Patch now removed. Thanks for taking a look! Release: 0.2%{?dist}
Why still leading 0 in release tag, is 1.3.0 a beta release?
You have to escape the macro in changelog:
%install -> %%install
Spec URL: http://supervacuo.com/fedora/grc.spec SRPM URL: http://supervacuo.com/fedora/grc-1.3-1.fc12.src.rpm Updated to fix issues described. Some quick comments: - The source code contains a man page. The rpm is missing that man page. - There is a mixed use of spaces and tabs. - Please have a lot at https://fedoraproject.org/wiki/Packaging:Python - Please get in touch with upstream about the license. Any updates here? I'm working on the issues described in comment 5; apologies for the delay. Fabian, is there any chance you could point out which parts of https://fedoraproject.org/wiki/Packaging:Python you feel I've missed? Finally had some time to revisit this. New package: Spec URL: http://supervacuo.com/fedora/grc.spec SRPM URL: http://supervacuo.com/fedora/grc-1.4-1.fc16.src.rpm (In reply to comment #5) > - The source code contains a man page. The rpm is missing that man page. Man pages now included > - There is a mixed use of spaces and tabs. Fixed. > - Please have a lot at https://fedoraproject.org/wiki/Packaging:Python I've read it thoroughly, and I'm not sure it's relevant. Although grc is written in Python, it is not (yet?) implemented as a Python module: it does not need to know where python_sitelib is, or do any byte-compilation. My interpretation is therefore that no changes are needed: I'd appreciate your advice if this is not so. > - Please get in touch with upstream about the license. COPYING is now also correctly included. The license is GPLv2 with some additions: one of them is that it can be re-licensed under any licence fulfilling Debian's guidelines. Should I leave it as-is, or change it to GPL? I've also updated to the latest upstream release, and am bypassing the included `install.sh` because it does not include all necessary files, anyway. (In reply to comment #8) > > - Please have a lot at https://fedoraproject.org/wiki/Packaging:Python > I've read it thoroughly, and I'm not sure it's relevant. Although grc is > written in Python, it is not (yet?) implemented as a Python module: it does not > need to know where python_sitelib is, or do any byte-compilation. My > interpretation is therefore that no changes are needed: I'd appreciate your > advice if this is not so. Sorry, it seams that I messed up two review requests. Hi Carl, still interest in this package? There is 1.5 release available upstream. Are you sponsored? @Buland, are you going to start the review any time soon? Terje, thanks for asking. I think I should back away from this package for the following reasons: * other packages like ccze are available in the repos and I can't really see features that grc adds (I wanted it because I was coming from Ubuntu and didn't want to rewrite my bash aliases...) * as Christopher Meng notes, there's a name clash with the GNU Radio GUI -- I don't see an easy way of resolving this * I don't know how to go about byte-compiling the Python scripts in the spec.. if that's indeed required (I also don't know how to tell if it is!) If anyone can advise on the name or packaging issue, though, I am happy to finish off the job just for the sake of completeness. |