Bug 847811
| Summary: | Review Request: libee - An event expression library | ||
|---|---|---|---|
| Product: | [Fedora] Fedora | Reporter: | mahaveer darade <mdarade> |
| Component: | Package Review | Assignee: | Tomas Mraz <tmraz> |
| Status: | CLOSED ERRATA | QA Contact: | Fedora Extras Quality Assurance <extras-qa> |
| Severity: | medium | Docs Contact: | |
| Priority: | medium | ||
| Version: | rawhide | CC: | andrew, bugs.michael, fdinitto, lemenkov, lhh, mah.darade, mitr, nobody+PNT0469646, notting, package-review, theinric, tmraz, vgaikwad |
| Target Milestone: | --- | Flags: | tmraz:
fedora-review+
j: fedora-cvs+ |
| Target Release: | --- | ||
| Hardware: | All | ||
| OS: | Linux | ||
| Whiteboard: | |||
| Fixed In Version: | Doc Type: | Bug Fix | |
| Doc Text: | Story Points: | --- | |
| Clone Of: | Environment: | ||
| Last Closed: | 2012-12-16 09:36: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: | 847817 | ||
| Bug Blocks: | 848388 | ||
|
Description
mahaveer darade
2012-08-13 15:52:24 UTC
If you set fedora-review, nobody will see your tickets. Hi, this is just an informational review.
1. use macros and variables consistently
2. %clean is not necessary
3. i'm not sure about %{_sbindir}/libee-convert, it could go in separate -utils package
Regards,
Milan
Done. I have worked on all 3 suggestions and changes are in-place. Below is new SRPM link: http://siddharths.fedorapeople.org/mahaveer/SRPMS/libee-0.4.1-2.fc15.src.rpm Below are new rpmlint logs. [root@mdarade guest]# rpmlint -i libee.spec 0 packages and 1 specfiles checked; 0 errors, 0 warnings. [root@mdarade guest]# rpmlint -i libee-0.4.1-2.fc15.src.rpm 1 packages and 0 specfiles checked; 0 errors, 0 warnings. [root@mdarade guest]# rpmlint -i libee-*.x86_64.rpm libee-devel.x86_64: W: spelling-error %description -l en_US rsyslog -> serology The value of this tag appears to be misspelled. Please double-check. libee-devel.x86_64: W: no-documentation The package contains no documentation (README, doc, etc). You have to include documentation files. libee-utils.x86_64: W: no-documentation The package contains no documentation (README, doc, etc). You have to include documentation files. libee-utils.x86_64: W: no-manual-page-for-binary libee-convert Each executable in standard binary directories should have a man page. 4 packages and 0 specfiles checked; 0 errors, 4 warnings. [root@mdarade guest]# > Summary: An event expression library inspired by CEE Not covered by any guidelines, but in many cases the leading article is superfluous and doesn't increase conciseness. Summary: Event expression library inspired by CEE That makes a much better reading when displayed by Anaconda and package tools. > License: LGPLv2+ True. The majority of source files contain an LGPLv2+ preamble. Only src/cjson/* contains merged files with an MIT-style preamble. There's a minor typo at the top of file COPYING, which refers to GPL rather than LGPL. > Group: Development/Libraries Base library packages still enter group "System Environment/Libraries". > %description I would rearrange the two paragraphs to begin with the explanation of what libee does, then continue with the explanation of what CEE is. > %package devel > Summary: Include files for libee More generally, it's Summary: Development files for libee because not just the header files are included. > Requires: %name = %version-%release https://fedoraproject.org/wiki/Packaging:Guidelines#Requiring_Base_Package > %package utils > Summary: The libee-convert utility provided by event expression library > Requires: %{name} = %{version}-%{release} Same here. The summary restricts the package contents to just the libee-convert utility. It could be made more general to sum up whether these tools are optional or strictly needed. > %{__make} https://fedoraproject.org/wiki/Packaging:Guidelines#Parallel_make Plus: if you don't have a compelling reason to use a macro here, feel free to run "make" from $PATH just as done with many other commands one runs in .spec files. > %install > rm -rf %{buildroot} It will be automatically cleaned: https://fedoraproject.org/wiki/Packaging:Guidelines#BuildRoot_tag * As noticed in the related reviews, there are inter-package dependencies involved. libee-devel should contain
Requires: libestr-devel%{?_isa}
as headers like libee.h include libestr headers.
> %build
> %configure
> %{__make}
Calling "V=1 make" for more verbose build output would be nice. For this package, the linking stage would be affected, so a test-build was necessary to notice this.
> %{__make} install -p DESTDIR=%{buildroot}
That error is in here, too:
make install INSTALL="install -p" DESTDIR=%{buildroot}
* The "tests" directory contains a testsuite that's suitable for running it in a %check section in the package.
I have worked on all the comments provided by Michael and here is a link to updated package. http://siddharths.fedorapeople.org/mahaveer/SPECS/libee.spec http://siddharths.fedorapeople.org/mahaveer/SRPMS/libee-0.4.1-3.fc15.src.rpm I think the license tag is wrong - it should be probably either LGPLv2+ and MIT or LGPLv2+ alone. The statement at the beginning of COPYING file is probably an upstream mistake as the full text of the license is LGPL2.1 and not GPL. Please correct the licence tag and notify upstream about the mistake. I will approve the package anyway as the licence tag mistake can be fixed before import. rpmlint -v libee-0.4.1-3.fc16.src.rpm libee-0.4.1-3.fc16.x86_64.rpm libee-devel-0.4.1-3.fc16.x86_64.rpm libee-utils-0.4.1-3.fc16.x86_64.rpm libee.src: I: checking libee.src: I: checking-url http://www.libee.org (timeout 10 seconds) libee.src: I: checking-url http://www.libee.org/files/download/libee-0.4.1.tar.gz (timeout 10 seconds) libee.x86_64: I: checking libee.x86_64: I: checking-url http://www.libee.org (timeout 10 seconds) libee-devel.x86_64: I: checking libee-devel.x86_64: W: spelling-error %description -l en_US rsyslog -> serology libee-devel.x86_64: I: checking-url http://www.libee.org (timeout 10 seconds) libee-devel.x86_64: W: no-documentation libee-utils.x86_64: I: checking libee-utils.x86_64: I: checking-url http://www.libee.org (timeout 10 seconds) libee-utils.x86_64: W: no-documentation libee-utils.x86_64: W: no-manual-page-for-binary libee-convert 4 packages and 0 specfiles checked; 0 errors, 4 warnings. The spelling error is not a real error. Missing documentation and manual page is upstream issue. Tarball matches the upstream sources. The package complies with Fedora packaging and licensing guidelines. The package is ACCEPTED. > I think the license tag is wrong In comment 4 I explicitly acknowledge that "License: LGPLv2+" was correct. No idea why it has been changed in the newer spec file. The %changelog should have mentioned such changes to _the packaging_. That's one of the things packagers ought to practise. I've worked on comments by Michael & Tomas. Below are the links to updated files. http://mdarade.fedorapeople.org/SPECS/libee.spec http://mdarade.fedorapeople.org/SRPMS/libee-0.4.1-4.fc15.src.rpm Please do not change the COPYING file, it needs to be corrected upstream. Also the package is already ACCEPTED, so please follow the packaging process to build it into Fedora. New Package SCM Request ======================= Package Name: libee Short Description: Event expression library inspired by CEE Owners: mdarade Branches: f17 f18 InitialCC: Git done (by process-git-requests). libee-0.4.1-3.fc18 has been submitted as an update for Fedora 18. https://admin.fedoraproject.org/updates/libee-0.4.1-3.fc18 libee-0.4.1-3.fc18 has been pushed to the Fedora 18 testing repository. libee-0.4.1-3.fc17 has been submitted as an update for Fedora 17. https://admin.fedoraproject.org/updates/libee-0.4.1-3.fc17 |