Bug 459161 - Review Request: wraplinux - Utility to wrap a Linux kernel and initrd into an ELF or NBI file
Summary: Review Request: wraplinux - Utility to wrap a Linux kernel and initrd into an...
Keywords:
Status: CLOSED RAWHIDE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Rahul Sundaram
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks: FE-Legal
TreeView+ depends on / blocked
 
Reported: 2008-08-14 19:29 UTC by Warren Togami
Modified: 2013-03-13 05:43 UTC (History)
5 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2008-11-13 18:44:01 UTC
Type: ---
Embargoed:
sundaram: fedora-review+


Attachments (Terms of Use)

Description Warren Togami 2008-08-14 19:29:28 UTC
Spec URL: http://togami.com/~warren/fedora/wraplinux.spec
SRPM URL: http://togami.com/~warren/fedora/wraplinux-1.5-1.fc9.src.rpm

A tool to wrap an x86 Linux kernel and one or more initrd files into a
single file in ELF or NBI format, as required by some booting protocols.

Comment 1 Warren Togami 2008-08-14 20:30:40 UTC
As requested by drago01:
- Builds in mock (yes it really doesn't have any build reqs)
-  wraplinux.i386: W: executable-stack /usr/bin/wraplinux
This is the only rpmlint warning.  Asking hpa upstream if this was intended or not.

Comment 2 Warren Togami 2008-08-15 02:11:46 UTC
http://togami.com/~warren/fedora/wraplinux.spec
http://togami.com/~warren/fedora/wraplinux-1.6-1.fc9.src.rpm

No remaining rpmlint warnings, builds cleanly in mock.

Comment 3 Rahul Sundaram 2008-08-15 02:18:40 UTC

Looks clean. Approved.

Comment 4 Christoph Wickert 2009-07-19 15:30:00 UTC
Rahul, sorry to rain on your parade once again, but the license tag is wrong. It's GPLv2+, not GPLv2. You did check the headers, did you?

Warren, please fix this in the next version or rebuild. Thanks.

Comment 5 Rahul Sundaram 2009-07-19 15:40:57 UTC
I don't recall what all I checked six months backs and whether the headers have changed meanwhile in new releases but I am feeling quite amused you decided to recheck all my reviews. I suggest that if you find packaging bugs, you file a new bug report instead of commenting on closed reviews. I know package maintainers who filter out comments on closed bug reports to low priority or even /dev/null.

Comment 6 Christoph Wickert 2009-07-19 15:58:19 UTC
(In reply to comment #5)
> I don't recall what all I checked six months backs

One more reason to write it down in the review...

> and whether the headers have changed meanwhile in new releases

There was no new release in the meantime.

> but I am feeling quite amused you decided to
> recheck all my reviews. 

Not all your reviews, just the obvious errors that are easy to find.

> I suggest that if you find packaging bugs, you file a
> new bug report instead of commenting on closed reviews. 

Thanks for the suggestion, I will CC you then because I want you to become aware of your errors.

Comment 7 Rahul Sundaram 2009-07-19 16:16:14 UTC
Writing down makes no real difference to me. There has been instances of people blindly copy pasting templates and still overlooking checklist items. Again, instead of approaching it on a case by case basis, if you feel strongly about how reviews should be done, it is better to take it to the packaging committee or FESCo and make it part of the guidelines/rules. Otherwise it is just individual's freedom to do the reviews as they see fit. 

I would also note that packaging is primarily the responsibility of the package maintainers. Reviewers are merely volunteering to help out with the process as QA assistance. Mistakes are bound to happen either way even with a recheck. Just to demonstrate, I checked the copyright notices again, GPLv2+ doesn't seem the right choice here either. reloc_linux.c seems to be under the MIT license actually and xmalloc.c doesn't even have a license notice. Warren, I suggest you contact upstream and clarify these.

Comment 8 Warren Togami 2009-07-19 16:41:44 UTC
> It's GPLv2+, not GPLv2. You did check the headers, did you?

What if I choose to use it only under the GPLv2?

You know, I really don't care.  Aren't you a provenpackager?  Just fix it yourself if you care.  It really doesn't matter.


Note You need to log in before you can comment on or make changes to this bug.