[Bug 1060386] Review Request: pandorafms-agent - Pandora FMS Linux agent.

[bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1060386

Christopher Meng  changed:

   What|Removed |Added

Summary|Review Request: |Review Request:
   |pandorafms_agent_unix - |pandorafms-agent - Pandora
   |Pandora FMS Linux agent.|FMS Linux agent.
  Alias||pandorafms-agent



--- Comment #7 from Christopher Meng  ---
Ugly spec.

1. Remove this:

#
# spec file for package pandorafms-agent
#
# Pandora FMS Linux Agent
# Copyright (c) 2014 Sancho Lerena
# Licensed under GPL2 terms.
# Please send bugfixes or comments to slerena@xxx

Your email will be leaked to public for spammer.

2. %define namepandorafms-agent
%define version 5.0
%define release 140201.sp3

Please use proper tag, no need to define a macro to finish that.

3. Remove these:
Vendor: ArticaST 
Group:  System/Monitoring
Packager:   Sancho Lerena 
Prefix: /usr/share
BuildRoot:  %{_tmppath}/%{name}-%{version}-buildroot

For this one /usr/share, please

rpm -E %{_datadir}

4. rm -rf $RPM_BUILD_ROOT is not needed in %prep and %install

5. These are not done by command, should be done by RPM:

# Checking old config file (if exists)
if [ -f /etc/pandora/pandora_agent.conf ] ; then
mv /etc/pandora/pandora_agent.conf /etc/pandora/pandora_agent.conf.backup
fi

cp -aRf $RPM_BUILD_ROOT%{prefix}/pandora_agent/Linux/pandora_agent.conf
$RPM_BUILD_ROOT/usr/share/pandora_agent/pandora_agent.conf.rpmnew

if [ -f $RPM_BUILD_ROOT%{prefix}/pandora_agent/pandora_agent.spec ] ; then
rm $RPM_BUILD_ROOT%{prefix}/pandora_agent/pandora_agent.spec
fi

So remove them.

6. No %clean section please.

7. %pre is not fine:

https://fedoraproject.org/wiki/Packaging:UsersAndGroups#Dynamic_allocation

8. Please learn how to write changelog:

* Sat Feb 01 2014 - slerena@x
- First version, after re-re-re-reading the fedora contributor guidelines :)

Take a look at existing one:

http://pkgs.fedoraproject.org/cgit/nagios.git/tree/nagios.spec

God, totally a mess, please clean above at first, then step forward. I can help
you, but don't be hasty.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list
package-review@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review

[Bug 1060386] Review Request: pandorafms-agent - Pandora FMS Linux agent.

[bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1060386



--- Comment #8 from Michael Schwendt  ---
> Prefix: /usr/share

Defining %{prefix} via a Prefix tag would mark the package as being
relocatable. That's important to know, and the following applies:

  https://fedoraproject.org/wiki/Packaging:Guidelines#Relocatable_packages


Currently, the package contents are not relocatable, so overriding %prefix with
/usr/share is just a bad idea. Instead, use existing macros that inherit from
eachother, such as %_bindir, %_datadir, %_mandir.
https://fedoraproject.org/wiki/Packaging:Guidelines#Macros

[...]

> About the fedora-review process, I think that should be done by a reviewer,
> right?. 

Why would you not perform a self-review of your own package with the help of
tools like "rpmlint -i" (mandatory) and "fedora-review"? ;-)  How do you decide
whether you believe your package would pass review? You need a reviewer for the
approval of the package, but not for re-writing your spec file. If you haven't
installed/used Mock before, doing that would be good exercise.

Beyond that, all package submitters are expected to be able to know the
ReviewGuidelines and also do a few reviews of packages in the queue.

  https://fedoraproject.org/wiki/Category:Package_Maintainers
  https://fedoraproject.org/wiki/Package_Review_Process
  http://fedoraproject.org/PackageReviewStatus/


> http://code.pandorafms.com/files/pandorafms-agent.spec

I've examined the diff against the first version, and you haven't changed much.

Please start with running "rpmlint -i" on the src.rpm *and* all built rpms *at
once*. Get familiar with rpmlint errors/warnings. Feel free to ignore obvious
false positives in the report, but fix anything else. Preferably add a comment
here about whether/when you think what rpmlint reports is correct or incorrect.

The spec file does a lot of questionable/strange/unusual things without any
comments in the spec file. Some of it is not covered by the packaging
guidelines, because nobody would do such things. If you added
comments/explanations in your %install, %preun and %post sections, for example,
a reviewer and/or sponsor could understand _why_ you think you need to do the
stuff you do in those sections. As a start, please try to explain what you do
in %install, %preun and %post sections and why you do it in those sections.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list
package-review@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review

[Bug 1060386] Review Request: pandorafms-agent - Pandora FMS Linux agent.

[bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1060386



--- Comment #9 from Sancho Lerena  ---
Wow. Thanks for the information, I will have a busy weekend working on all of
these :)

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list
package-review@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review

[Bug 1060386] Review Request: pandorafms-agent - Pandora FMS Linux agent.

[bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1060386



--- Comment #10 from Sancho Lerena  ---
Hello again,

I tried to focus on rewrite the SPEC based on some suggestion and example
(nagios) spec. Thanks for the help.

The updated spec is at the same URL (overwriting the old one):

http://code.pandorafms.com/files/pandorafms-agent.spec

rpmlint passed ok on spec and SRPM. On binary RPM get lots of E/W and some of
them and frustrating :(

I have some problems (understanding the way it behaves) about the files
included in a directory /plugins which I copy manually (with CP). It works ok,
but rpmlink on binary RPM got lots of errors :(

Any advice here will be very appreciated. 

Thanks.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list
package-review@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review

[Bug 1060386] Review Request: pandorafms-agent - Pandora FMS Linux agent.

[bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1060386



--- Comment #11 from Christopher Meng  ---
OK.

Second stage(dont be panic, a lot still):

0. Why don't you use tarball at here: 

http://sourceforge.net/projects/pandora/files/Pandora%20FMS%205.0/FinalSP3/Tarball/pandorafms_agent_unix-5.0SP3.tar.gz

as source?

1. Vendor: Artica 

Remove this.

2. Please remove the lines, don't comment them.

3. No need to add these IMO: Requires(pre): /bin/sed /bin/grep 

4. Requires(preun): initscripts, chkconfig
Requires(post): initscripts, chkconfig
Requires(postun): initscripts

I'm sorry, please learn systemd, and write a service file:

https://fedoraproject.org/wiki/Packaging:Systemd

4. rm -rf %{buildroot} in %install section is not needed, delete it.

5. Please perserve the timestamp with install -p option:

6. %description is too poor, you are the upstream right? Why not improve it?

7. You didn't read comment 7 carefully, please remove %clean section.

8. changelog release field syntax incorrect.

Please rpmdev-bumpspec to bump the spec and see what will happen, then fix it.

9. Please learn and replace macros: 

https://fedoraproject.org/wiki/Packaging:RPMMacros

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list
package-review@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review

[Bug 1060386] Review Request: pandorafms-agent - Pandora FMS Linux agent.

[bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1060386



--- Comment #12 from Christopher Meng  ---
For issue 8, an extra question:

Release: 140223.sp3%{?dist}

Fedora packages seldom have release number up to 10^5, you'd better start from
1, but rebuild may destroy what you expect and may cause upgrade path issues,
therefore please take a look at here for package has non-numeric version:

https://fedoraproject.org/wiki/Packaging:NamingGuidelines#Package_Versioning

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list
package-review@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review

[Bug 1060386] Review Request: pandorafms-agent - Pandora FMS Linux agent.

[bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1060386



--- Comment #13 from Michael Schwendt  ---
Running "fedora-review -b 1060386" fails. Please keep the "Spec URL:" and "SRPM
URL:" lines up-to-date.

[...]

Which rpmlint warnings/error do you find frustrating?

Try "rpmlint -i …" on the built rpm. The -i adds helpful output. If you get too
many W/E, you can also query rpmlint for its help, e.g.

  $ rpmlint -I service-default-enabled
  service-default-enabled:
  The service is enabled by default after "chkconfig --add"; for security
  reasons, most services should not be. Use "-" as the default runlevel in the
  init script's "chkconfig:" line and/or remove the "Default-Start:" LSB
keyword
  to fix this if appropriate for this service.


  $ rpmlint -I conffile-without-noreplace-flag
  conffile-without-noreplace-flag:
  A configuration file is stored in your package without the noreplace flag. A
  way to resolve this is to put the following in your SPEC file:
  %config(noreplace) /etc/your_config_file_here

[...]

> pandorafms-agent.noarch: W: dangerous-command-in-%preun userdel

https://fedoraproject.org/wiki/Packaging:UsersAndGroups


> %post
> mkdir -p /var/spool/pandora/data_out

Why aren't these directories included in the package?


> %doc

An empty %doc does nothing.

> /usr/share/man/man1/pandora_agent.1.gz
> /usr/share/man/man1/tentacle_client.1.gz

$ rpm -E %_mandir
/usr/share/man

Typically, one uses wildcards and macros to include the files:

  %{_mandir}/man1/*.1*

Not specifying the trailing .gz is not a big issue, but allows for
disabling/changing/customizing compressing of manual pages without breaking the
spec file.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list
package-review@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review

[Bug 1060386] Review Request: pandorafms-agent - Pandora FMS Linux agent.

[bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1060386

Miroslav Suchý  changed:

   What|Removed |Added

 Status|NEW |CLOSED
 Blocks|177841 (FE-NEEDSPONSOR) |201449 (FE-DEADREVIEW)
 Resolution|--- |NOTABUG
  Flags|needinfo?(sler...@gmail.com |
   |)   |
Last Closed||2016-02-08 08:49:23



--- Comment #16 from Miroslav Suchý  ---
No response. Closing as dead review. If you ever want to continue, please
resubmit.


Referenced Bugs:

https://bugzilla.redhat.com/show_bug.cgi?id=177841
[Bug 177841] Tracker: Review requests from new Fedora packagers who need a
sponsor
https://bugzilla.redhat.com/show_bug.cgi?id=201449
[Bug 201449] FE-DEADREVIEW -- Reviews stalled due to lack of submitter
response should be blocking this bug.
-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list
package-review@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review

[Bug 1060386] Review Request: pandorafms-agent - Pandora FMS Linux agent.

[bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1060386

Miroslav Suchý  changed:

   What|Removed |Added

 CC||msu...@redhat.com,
   ||sler...@gmail.com
  Flags||needinfo?(sler...@gmail.com
   ||)



--- Comment #14 from Miroslav Suchý  ---
Sancho, any progress here? Are you still interrested in this review?

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list
package-review@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review

[Bug 1060386] Review Request: pandorafms-agent - Pandora FMS Linux agent.

[bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1060386



--- Comment #15 from Upstream Release Monitoring 
 ---
pbrobinson's scratch build of
linux-user-chroot?#b7afe5173cbd31b029b027b6f8a14baa5e6ce87a for
epel7-archbootstrap and
git://pkgs.fedoraproject.org/linux-user-chroot?#b7afe5173cbd31b029b027b6f8a14baa5e6ce87a
failed http://koji.fedoraproject.org/koji/taskinfo?taskID=12089939

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list
package-review@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review