[Bug 226322] New: Merge Review: psmisc

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug report.

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

Summary: Merge Review: psmisc
           Product: Fedora Extras
           Version: devel
          Platform: All
        OS/Version: Linux
            Status: NEW
          Severity: normal
          Priority: normal
         Component: Package Review
        AssignedTo: [e-mail]
        ReportedBy: [e-mail]
         QAContact: [e-mail]
                CC: [e-mail]

Fedora Merge Review: psmisc

http://cvs.fedora.redhat.com/viewcvs/devel/psmisc/
Initial Owner: [e-mail]

[Bug 226322] Merge Review: psmisc

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug report.

Summary: Merge Review: psmisc

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

[e-mail] changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
               Flag|                            |fedora-review?

[Bug 226322] Merge Review: psmisc

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug report.

Summary: Merge Review: psmisc

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

[e-mail] changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |[e-mail],
                   |                            |[e-mail]

------- Additional Comments From [e-mail]  2007-02-03 16:24 EST -------
I reviewed this one.  Just a few things:

rpmlint on ./psmisc-22.2-5.src.rpm
W: psmisc summary-ended-with-dot Utilities for managing processes on your system.
W: psmisc macro-in-%changelog _includedir

Please use the %{dist} in release

BuildRequires should require gettext rather than gettext-devel

Otherwise, it looks good.

[Bug 226322] Merge Review: psmisc

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug report.

Summary: Merge Review: psmisc

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

[e-mail] changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
         AssignedTo|[e-mail]    |[e-mail]
               Flag|fedora-review?              |fedora-review-

------- Additional Comments From [e-mail]  2007-02-04 13:15 EST -------
Andy, please reassign tickets back to the requester and switch the fedora-review flag to
-, if you deny the request
+, if you approve it.

See http://www.fedoraproject.org/wiki/WarrenTogami/ReviewWithFlags for more info.

[Bug 226322] Merge Review: psmisc

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug report.

Summary: Merge Review: psmisc

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

------- Additional Comments From [e-mail]  2007-02-05 12:07 EST -------
Ruben, sorry for not following procedure.  I should have explained that I'm not
sponsored yet, so I do not have access to reassign tickets.

[Bug 226322] Merge Review: psmisc

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug report.

Summary: Merge Review: psmisc

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

------- Additional Comments From [e-mail]  2007-03-01 09:48 EST -------
Fixed. Update to psmisc-22.3-1.fc7. Thanks for your review.

[Bug 226322] Merge Review: psmisc

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug report.

Summary: Merge Review: psmisc

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

[e-mail] changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
           Severity|normal                      |medium

[e-mail] changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED

[Bug 226322] Merge Review: psmisc

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug report.

Summary: Merge Review: psmisc

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

[e-mail] changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
           Priority|normal                      |medium
            Product|Fedora Extras               |Fedora
            Version|devel                       |rawhide

[e-mail] changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
OtherBugsDependingO|                            |426387
              nThis|                            |

[Bug 226322] Merge Review: psmisc

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.

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

Jon Ciesla  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |[e-mail],
                   |                            |[e-mail]

--- Comment #5 from Jon Ciesla   2008-09-18 10:11:42 EDT ---
Adding current owner.

Karel, what is the status here?  I see you are the previous owner.  Any
objection to my taking ownership of the bug, and completing the review?

[Bug 226322] Merge Review: psmisc

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.

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

Karel Zak  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
         AssignedTo|[e-mail]             |[e-mail]

[Bug 226322] Merge Review: psmisc

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.

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

--- Comment #6 from Daniel Novotny   2008-09-19 06:55:13 EDT ---
Jon, I am the new maintainer of psmisc. You can take the ownership and complete
the review.

[Bug 226322] Merge Review: psmisc

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.

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

Jon Ciesla  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
         AssignedTo|[e-mail]         |[e-mail]

--- Comment #7 from Jon Ciesla   2008-09-19 08:35:31 EDT ---
Excellent.  I'll get right on it.

[Bug 226322] Merge Review: psmisc

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.

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

--- Comment #8 from Jon Ciesla   2008-09-19 09:00:44 EDT ---
Full review is stellar.

One think I might ask, and it's not worth a rebuild, but could you comment in
the spec on the purpose and upstream status of the patches?  Just got added to
the Guidelines and I think it's a good practice.

[Bug 226322] Merge Review: psmisc

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.

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

--- Comment #9 from Jon Ciesla   2008-12-04 14:35:34 EDT ---
Ping?

[Bug 226322] Merge Review: psmisc

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.

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

--- Comment #10 from Jon Ciesla   2009-03-31 11:26:14 EDT ---
Ping again?

[Bug 226322] Merge Review: psmisc

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.

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

Michal Nowak  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |[e-mail]
               Flag|                            |needinfo?(dnovotny@redhat.c
                   |                            |om)

--- Comment #11 from Michal Nowak   2009-04-08 12:41:51 EDT ---
Daniel, can you please follow up to Jon's proposals?

--

Also:

* Not necessary to define globally when used only for make

export CFLAGS="$RPM_OPT_FLAGS -D_GNU_SOURCE"

also not sure whether is "-D_GNU_SOURCE" necessary, shouldn't $RPM_OPT_FLAGS
take care of everything?

* don't mix variable and macro style

https://fedoraproject.org/wiki/Packaging/Guidelines#Using_.25.7Bbuildroot.7D...

* %defattr(-,root,root)

A little bit old-school to me. Use

%defattr(-,root,root,-)

* point URL field to http://sourceforge.net/projects/psmisc/ the present one is
useless (bogus content there)

[Bug 226322] Merge Review: psmisc

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.

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

Daniel Novotny  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
               Flag|needinfo?(dnovotny@redhat.c |
                   |om)                         |

--- Comment #12 from Daniel Novotny   2009-05-06 09:58:21 EDT ---
hello,

I made the suggested .spec changes:

* added purpose comments to all patches
 * fixed URL
 * fixed %defattr

as for the others:
> Not necessary to define globally when used only for make

configure and autoreconf doesn't use those? I'm not sure...
> don't mix variable and macro style

as far as I can see, there's only variable style for
those two variables

output: 
http://people.fedoraproject.org/~dnovotny/psmisc.review.spec

[Bug 226322] Merge Review: psmisc

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.

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

--- Comment #13 from Michal Nowak   2009-05-06 10:12:59 EDT ---
(In reply to comment #12)
> [...]
> output: 
> http://people.fedoraproject.org/~dnovotny/psmisc.review.spec

Looks good to me. (Don't forget tu bump changelog entry when in CVS, please.)

Jon: Are you satisfied? Will you approve it, or should I?

[Bug 226322] Merge Review: psmisc

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.

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

Jon Ciesla  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |CLOSED
         Resolution|                            |ERRATA
               Flag|fedora-review?              |fedora-review+

--- Comment #14 from Jon Ciesla   2009-05-07 12:57:52 EDT ---
I'm happy.  Commit.

APPROVED.

Thanks everyone!