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 217275
Summary: | Review Request: ocsinventory-client - Open Computer and Software Inventory Next Generation client | ||
---|---|---|---|
Product: | [Fedora] Fedora | Reporter: | Remi Collet <fedora> |
Component: | Package Review | Assignee: | Patrice Dumas <pertusus> |
Status: | CLOSED NEXTRELEASE | QA Contact: | Fedora Package Reviews List <fedora-package-review> |
Severity: | medium | Docs Contact: | |
Priority: | medium | ||
Version: | rawhide | CC: | pertusus |
Target Milestone: | --- | ||
Target Release: | --- | ||
Hardware: | All | ||
OS: | Linux | ||
Whiteboard: | |||
Fixed In Version: | Doc Type: | Bug Fix | |
Doc Text: | Story Points: | --- | |
Clone Of: | Environment: | ||
Last Closed: | 2006-12-02 08:06: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: | |||
Bug Depends On: | |||
Bug Blocks: | 163779 |
Description
Remi Collet
2006-11-26 10:34:38 UTC
* I think that in the comment explaining what ocstag and ocsserver I think that you should put a reference to the README. Something along # Can, optionaly, be defined at build time (see README) * instead of dos2unix, you can use sed -i 's/\r//' And similarly you can use sed instead perl to do the simple substitution, like sed -i -e 's|PATH_TO_LOG_DIRECTORY|%{_localstatedir}/log/%{name}|' logrotate.ocsinventory-client * I think that the README.fedora should be split in a README.fedora and README.fedora.fr, tagged like %lang(fr) %doc README.fedora.fr Patrice, Thanks for the comments Spec URL: http://remi.collet.free.fr/rpms/extras/ocsinventory-client.spec SRPM URL: http://remi.collet.free.fr/rpms/extras/ocsinventory-client-1.0-0.2.RC3.fc7.src.rpm %changelog * Sun Nov 26 2006 Remi Collet <Fedora> 1.0-0.2.RC3 - replace perl and dos2unix by sed - split README.fedora * %{buildroot} is used everywhere, so I guess $RPM_OPT_FLAGS should be changed to %{optflags}. * there are some unowned directory issues, %{_sysconfdir}/logrotate.d/ and %{_sysconfdir}/cron.daily/ (owned by logrotate and crontabs). There was a thread about that issue on fedora-devel-list, I have no particular opinion on what is the right solution. * from looking in the script, seems like there is a missing Requires: perl(HTTP::Request) > there are some unowned directory issues, Yes. I don't have real opinion too. logrotate and cron are installed on must system. I don't like to add an require for them as alternatives exists and "virtual-requires" not. Spec URL: http://remi.collet.free.fr/rpms/extras/ocsinventory-client.spec SRPM URL: http://remi.collet.free.fr/rpms/extras/ocsinventory-client-1.0-0.3.RC3.fc7.src.rpm * Mon Nov 27 2006 Remi Collet <Fedora> 1.0-0.3.RC3 - $RPM_OPT_FLAGS changed to %%{optflags}. - add not detected Requires perl(HTTP::Request) P.S. I will be away for the next 2 days... I will also change BuildRequires: dmidecode Requires: dmidecode to : BuildRequires: %{_sbindir}/dmidecode Requires: %{_sbindir}/dmidecode To allow build on older version (RHEL and FC) as dmidecode is provided by kernel-utils. (In reply to comment #4) > > there are some unowned directory issues, > > Yes. I don't have real opinion too. > logrotate and cron are installed on must system. I don't like to add an require > for them as alternatives exists and "virtual-requires" not. In that case you can add a Requires on the directories, or have ocsinventory-client own those directories, until they get owned by the filesystem package. (In reply to comment #5) > I will also change > BuildRequires: dmidecode > Requires: dmidecode > to : > BuildRequires: %{_sbindir}/dmidecode > Requires: %{_sbindir}/dmidecode > > To allow build on older version (RHEL and FC) as dmidecode is provided by > kernel-utils. Fine for me. > In that case you can add a Requires on the directories, or have > ocsinventory-client own those directories, until they get owned by > the filesystem package. Is this a MUST for the review ? I look at some core package and don't see anything... Some improvement to have ocsinventory-client build and work on RHEL and older FC (i really need this at work). Spec URL: http://remi.collet.free.fr/rpms/extras/ocsinventory-client.spec SRPM URL: http://remi.collet.free.fr/rpms/extras/ocsinventory-client-1.0-0.4.RC3.fc7.src.rpm * Tue Nov 28 2006 Remi Collet <Fedora> 1.0-0.4.RC3 - requires %%{_sbindir}/dmidecode (kernel-utils in FC3) rather than dmidecode - requires perl(:MODULE_COMPAT) only on Fedora (not provided on RHEL3) - patch improved (Fedora is RPM based) (In reply to comment #7) > > In that case you can add a Requires on the directories, or have > > ocsinventory-client own those directories, until they get owned by > > the filesystem package. > > Is this a MUST for the review ? Yes. I make exception for /usr/share/icons/hicolor. In the packaging guidelines it is at http://fedoraproject.org/wiki/Packaging/Guidelines#head-a5931a7372c4a00065713430984fa5875513e6d4 And in that case I think it is relevant, since it materializes whether you want to have a real dependency on logrotate/cron or make logrotate/cron optional - which means that the package owns the directory. > I look at some core package and don't see anything... Most core packages haven't gone through the guidelines and lacks the fedora extras packages quality. > Some improvement to have ocsinventory-client build and work on RHEL and older FC > (i really need this at work). With EPEL, this is indeed something acceptable or even encouraged. Besides in that case it doesn't really hurt legibility. > Spec URL: http://remi.collet.free.fr/rpms/extras/ocsinventory-client.spec > SRPM URL: > http://remi.collet.free.fr/rpms/extras/ocsinventory-client-1.0-0.4.RC3.fc7.src.rpm > > * Tue Nov 28 2006 Remi Collet <Fedora> 1.0-0.4.RC3 > - requires %%{_sbindir}/dmidecode (kernel-utils in FC3) rather than dmidecode > - requires perl(:MODULE_COMPAT) only on Fedora (not provided on RHEL3) > - patch improved (Fedora is RPM based) Apart from directory owning, seems right. Notice that I didn't assign the bug to myself, so a reviewer accepting unowned directories for those directories can step up. Spec URL: http://remi.collet.free.fr/rpms/extras/ocsinventory-client.spec SRPM URL: http://remi.collet.free.fr/rpms/extras/ocsinventory-client-1.0-0.5.RC3.fc7.src.rpm %changelog * Wed Nov 29 2006 Remi Collet <Fedora> 1.0-0.5.RC3 - Requires %%{_sysconfdir}/logrotate.d and %%{_sysconfdir}/cron.daily - define perl_vendorlib on non-fedora (for RHEL3) * rpmlint gives an ignorable warning, % in post is used in date invocation: W: ocsinventory-client percent-in-%post * the tarball name cannot be used, the name chosen is relevant, and used in other distros. * free software, license included * follow packaging guidelines * source match upstream: 34edd057f1937245d06c3515c0ff50ad OCSNG_LINUX_AGENT_1.0RC3.tar.gz * sane provides: Provides: config(ocsinventory-client) = 1.0-0.5.RC3 perl(Ocsinventory::Agent) perl(Ocsinventory::Agent::Common) perl(Ocsinventory::Agent::Option::Download) perl(Ocsinventory::Agent::Option::Ipdiscover) perl(Ocsinventory::Agent::Option::Update) * buildrequires right * %files section right APPROVED |