[Bug 968339] Review Request: ps_mem - Memory profiling tool

[bugzilla at redhat.com]

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

--- Comment #4 from Jaromír Cápík <jcapik at redhat.com> ---
Hi Greg.

Few reactions to your notes:

Defining the commit hash in form of global is unfortunately more work for the
mainainer, since it is easier to simply copy'n'paste the whole link when
browsing the scripts in the raw mode. As any additional line makes the spec
file longer and less synoptic, I would try to avoid globals in this case.

The mentioned description "Show process memory usage" unfortunately goes
contrary to the main behavioral difference against other ps tools. The purpose
of this script is to provide users with memory usage per application (not per
process) and that's why I see this as an inconsistence even when it is printed
directly by the script.

According to the following bug, the fedora-review issue has been fixed. Please
update to the fixed package and recheck.
https://bugzilla.redhat.com/show_bug.cgi?id=967571

Regarding timestamps. It's good to preserve timestamps of the files stored in
the upstream archive, but in this case the file was downloaded directly without
consequent unpacking and the timestamp was invalidated during the download
phase even when I tried to use the wget -N / curl -R options. I rechecked that
few minutes ago and I'm always getting today's date, so it seems the server
doesn't offer this capability. The previous timestamp was incorrect due to a
strange clock skew in my virtual machine. There's no reason for preserving
invalid timestamps. Anyway, I see this issue as minor and don't want it to
block the script introduction.

Regarding the EPEL stuff. At the moment I'm waiting for PM to decide if the
script is worthy for being included in the official branches and therefore I
don't want to introduce this package with explicit EPEL5/6 artifacts which make
the spec file more dirty for non-EPEL Fedora branches.

If you have no objections, please, do the review as if it was a non-EPEL
package. I'll add the EPEL stuff later if it is needed, since that would make
the merging between non-EPEL and EPEL branches easier.

Thanks and regards,
Jaromir.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
Unsubscribe from this bug https://bugzilla.redhat.com/token.cgi?t=o2WBJDaC03&a=cc_unsubscribe