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=199780
Summary: Review Request: dstat Product: Fedora Extras Version: devel Platform: All OS/Version: Linux Status: NEW Severity: normal Priority: normal Component: Package Review AssignedTo: bugzilla-sink@leemhuis.info ReportedBy: scott@perturb.org QAContact: fedora-package-review@redhat.com
Spec URL: http://www.perturb.org/tmp/dstat-fedora.spec SRPM URL: http://www.perturb.org/tmp/dstat-0.6.3-2.src.rpm Description: Versatile resource statistics tool
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: dstat
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=199780
toshio@tiki-lounge.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |toshio@tiki-lounge.com
------- Additional Comments From toshio@tiki-lounge.com 2006-07-21 18:00 EST ------- Okay. To start, a few links:
[1]_ General Guidelines the package must follow: http://www.fedoraproject.org/wiki/Packaging/Guidelines [2]_ Python Specific Packaging Hints: http://www.fedoraproject.org/wiki/Packaging/Python [3]_ Fedora Extras CVS. Look for examples of how other people do things here: http://cvs.fedora.redhat.com/viewcvs/rpms/?root=extras
Some python applications you can look through there are qa-assistant, rpmlint, and gramps.
The first thing that leaps out at me when I look at the spec is that the python files aren't bytecompiled. In addition to installing, you want to generate the .pyc and .pyo byte compiled files and install them. That way the package will know that it needs to remove the files as well.
[1] and [3] should help you get through that. Looking over a spec or two from [3] and comparing to your rpm would be another good thing to do.
(And now you can jump back on IRC and ask specific questions and we'll be better able to help ;-)
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: dstat
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=199780
------- Additional Comments From devrim@commandprompt.com 2006-07-21 18:09 EST ------- The spec file should be dstat.spec, not dstat-fedora.spec. Also setup needs -q .
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: dstat
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=199780
------- Additional Comments From scott@perturb.org 2006-07-21 18:40 EST ------- Added setup -q and renamed spec file to dstat.spec
http://www.perturb.org/tmp/dstat.spec
I will look into the python compiling to bytecode etc.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: dstat
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=199780
------- Additional Comments From scott@perturb.org 2006-07-21 19:29 EST ------- Updated the spec and the src RPM to bytecode compile the python.
Spec URL: http://www.perturb.org/tmp/dstat.spec SRPM URL: http://www.perturb.org/tmp/dstat-0.6.3-2.src.rpm
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: dstat
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=199780
toshio@tiki-lounge.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED AssignedTo|bugzilla-sink@leemhuis.info |toshio@tiki-lounge.com OtherBugsDependingO|163776 |163778, 177841 nThis| |
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: dstat
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=199780
------- Additional Comments From scott@perturb.org 2006-07-24 11:34 EST ------- What's the next step in getting this reviewed?
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: dstat
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=199780
------- Additional Comments From paul@all-the-johnsons.co.uk 2006-07-24 11:39 EST ------- Whoever is assigned to this bug reviews the fixes you've made, test builds, runs rpmlint over the packages created, compiles in mock, installs and runs rpmlint over the installed package to ensure nothing untoward happens.
This isn't an instant process - some bugs can be open for ages!
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: dstat
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=199780
------- Additional Comments From paul@all-the-johnsons.co.uk 2006-07-24 11:45 EST ------- Looking at the spec file you've got there, I can't see anything which bytecode compiles the python. Remember a # before anything is just counted as a comment and rpmbuild skips past it as if it wasn't there.
A couple of other niggles
In the Source: you have .../dstat/dstat-%{version}...
This can be substituted with .../%{name}/%{name}-%{version}. I know on packages I've had reviewed on, this has been brought up.
The make install line can just be
make DESTDIR=%{buildroot} install - you don't need the quotes around the buildroot
Does the package come with any language translation files?
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: dstat
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=199780
------- Additional Comments From toshio@tiki-lounge.com 2006-07-24 13:55 EST ------- Quick note as I start reviewing:
You should always bump the release number, even when you are making new spec files for review. This lets me easily reference the -2 release as having Problem A) and the -3 release fixed that but introduced problem B). It makes it easier to figure out what problems are still outstanding against a package and which have been fixed. Thanks.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: dstat
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=199780
------- Additional Comments From toshio@tiki-lounge.com 2006-07-25 15:47 EST ------- MD5sums: 72917aa5eed385464d70ec731bd6d2b1 dstat-0.6.3-2.src.rpm a2df5d7fecc0115f8eef84141a068e86 dstat-0.6.3.tar.bz2
Blocker: * Your patch adds #!/usr/bin/python to the modules files. Since the modules are not intended to be run from the command line, this is improper behaviour. You should make the files non-executable instead. * It's customary to put the defattr line first in the files section. I believe that rpm uses the defattr line even on lines that preceed it in the file listings but I don't know if that's documented behaviour. If that behaviour ever changes, then you could suddenly have files with incorrect ownership and permissions. * Remove the execute permissions from the scripts in the examples directory. * Package needs to own the dstat directory like this: %dir %{_datadir}/dstat
Cosmetic: * It looks like you're patching out dos line endings for html files. This is okay if you're going to submit the change upstream. As a general rule (for instance, if upstream were to not accept the change and you had to continue to carry it around in the package) it's better to use sed or dos2unix to fix this. Otherwise your patch becomes filled with whole files where the only difference is the EOL character. * Some people still use the default rpm defined topdir, sourcedir, etc. When this is so, installing the source rpm places the files in the same directory as a multitude of other rpms. So your patch: patch-to-clean-for-fedora.patch should really have a less generic name. Something like dstat-eol.patch or dstat-eol-cleanup.pach. * I'd remove the commented out code as it doesn't add any value to the spec. * You don't need to Require: python; this is being picked up automatically.
Good: * Source matches upstream. * Package Name follows the Naming Guidelines. * Spec file follows the %{name}.spec format. * License is GPL, matches the license field, and is included in the rpm. * Package built on x86_64 as a noarch package. * All BuildRequires not listed in the exceptions have been satisfied. * No locale files, so no need to use %find_lang. * No libraries or pkgconfig files. * Package is not relocatable. * No duplicate files. * Package has a proper %clean section. * Package uses proper macros from Packaging Guidelines. * Code, not content. * No large doc files. * Doc files do not affect the runtime behaviour. * Not a GUI application. * Does not own directories that belong to another package. * No scriptlets. * Package built in mock.
After you fix the listed issues, I can run a last rpmlint check and make sure the program runs and then APPROVE the package.
Since you need a sponsor, I need to know that you understand the guidelines before SPONSORing you. This package was a good indication of understanding on your part. If you do a review of someone else's package that continues to demonstrate your knowledge of the Fedora Guidelines in particular and rpm packaging in general, I'll sponsor you as well.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: dstat
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=199780
------- Additional Comments From scott@perturb.org 2006-07-25 17:24 EST ------- Ok I bumped the release number to 3, and re-uploaded it. I removed my patch, and did the work in the spec file. Sed is used to convert the documentation line endings, and I chmoded the .py as requested. Moved defattr to the top, and added the %dir lines. Commented out code was removed as was the require python.
rpmlint is only giving out warnings about the line endings as well as the a stale symlink. I've done my best to handle that in the .spec file (see the rm and sed lines) but it's just not working. I'm over my head.
SPEC: http://www.perturb.org/tmp/dstat.spec SRC: http://www.perturb.org/tmp/dstat-0.6.3-3.src.rpm
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: dstat
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=199780
------- Additional Comments From toshio@tiki-lounge.com 2006-07-25 19:01 EST ------- * The files don't get moved into _docdir until after %install. So instead of operating on %{buildroot}/%{_docdir}/* you want to operate on the files in your source tree.
* You've commented out the %clean section. You'll want to put that back. * There are other pieces of commented code that could be removed: #BuildRequires:.. #Requires:.... #/usr/lib/rpm/brp-python-bytecompile.... #%config(noreplace) %{_sysconfdir}/dstat.conf....
You can leave them in with explanations if you think they make the spec clearer and easier to understand (I see that you have commented on two of those listed already. The choice is yours.)
* For every release bump, there must also be a changelog entry. So instead of just bumping the release number on the changelog line, add a whole new entry that tells what you've changed.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: dstat
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=199780
------- Additional Comments From scott@perturb.org 2006-07-31 13:19 EST ------- Oh lame... I posted last week and it didn't make it.
Spec: http://www.perturb.org/tmp/dstat.spec SRPM: http://www.perturb.org/tmp/dstat-0.6.3-4.src.rpm
- Removed some commeted lines in the .spec file that weren't needed - Changed the permissions on the examples/* scripts - Converted the HTML documentation to unix line endings - Removed the erroneous commenting of the %clean section
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: dstat
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=199780
------- Additional Comments From toshio@tiki-lounge.com 2006-08-05 14:33 EST ------- MD5sums: ed346ba8fb71514e5be2202d50a17568 ../dstat-0.6.3-4.src.rpm a2df5d7fecc0115f8eef84141a068e86 dstat-0.6.3.tar.bz2 0862411cacd97a31182dd2b0199c512f dstat.spec
Most previously noted problems fixed. Here's what remains: * Remove the execute permissions from the scripts in the examples directory. * macros (Like %clean) are expanded even when they're in the changelog section. You'll want to escape the macro in your changelog with an additional "%":: - Removed the erroneous commenting of the %%clean section
After you fix these issues, I can do a last check and APPROVE the package. You will also need to be sponsored so you can import the package into the repository. I am willing to do this if you show your understanding of the packaging guidelines. This is best done by reviewing someone else's package according to the packaging guidelines and lettting me know so I can watch the process. You can refer to these two documents for help::
http://www.fedoraproject.org/wiki/Extras/HowToGetSponsored http://www.fedoraproject.org/wiki/Packaging/ReviewGuidelines
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: dstat
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=199780
------- Additional Comments From scott@perturb.org 2006-08-11 17:00 EST ------- SPEC: http://www.perturb.org/tmp/dstat.spec SRPM: http://www.perturb.org/tmp/dstat-0.6.3-5.src.rpm
I fixed the above mentioned things. rpmlint is clean on both the srpm and the binary rpm. I don't know how much time I'll have to review another project however. Plus I'm not sure I have the expertise.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: dstat
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=199780
------- Additional Comments From toshio@tiki-lounge.com 2006-08-17 12:10 EST ------- e3bc24955e52a78166ff297f1a5e14ee dstat-0.6.3-5.src.rpm f833f3e0f8bb34ed50bf1388b7281cb7 dstat.spec a2df5d7fecc0115f8eef84141a068e86 dstat-0.6.3.tar.bz2
You can get rid of this line if you'd like: /usr/lib/rpm/brp-python-bytecompile # FC4 and FC5 run this automatically
as you've noted, it gets run automatically on FC4+. If you are building for FC3, that script won't be present at all so the spec file will fail to build.
All blockers have been resolved. I'm ready to ACCEPT this package, we just need to get you sponsored.
As for reviewing and sponsorship. You get sponsored after you show an understanding for the guidelines. Understanding of the guidelines is all it takes to be able to do reviews. And since you're in the sponsorship process, I'll be watching over your shoulder so I can correct anything that you do wrong. (And you already know how to get into #fedora-extras on IRC which is a great resource for asking questions that might come up during the review process.)
Basically, once you are sponsored, you will be able to make changes to any package in the CVS tree and be able to approve other people's packages. So having some examples showing you have the knowledge to do that well is essential.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: dstat
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=199780
------- Additional Comments From scott@perturb.org 2006-08-28 12:35 EST ------- So what's next? All I want to do is get this package in the extras tree.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: dstat
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=199780
------- Additional Comments From paul@all-the-johnsons.co.uk 2006-08-28 13:04 EST ------- You need sponsorship. The best way to do that is submit a couple of packages then those who have the power to do so can correctly evaluate if you understand the packaging rules.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: dstat
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=199780
------- Additional Comments From toshio@tiki-lounge.com 2006-08-28 13:17 EST ------- (In reply to comment #16)
So what's next? All I want to do is get this package in the extras tree.
And qcomicbook? :-)
You can also do reviews.
This is explained in Comment #13, Comment #15, and http://www.fedoraproject.org/wiki/Extras/HowToGetSponsored
If you'd like, I can unassign this bug and you can seek someone else on fedora-extras to sponsor you. They'll probably review qcomicbook and look at this bug to judge how knowledgable you are and then decide if they want you to do reviews or anything else to show your knowledge. Your call.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: dstat
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=199780
------- Additional Comments From scott@perturb.org 2006-08-28 13:19 EST ------- I'd prefer not to do reviews, I'm not sure if my knowledge is ready for that.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: dstat
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=199780
toshio@tiki-lounge.com changed:
What |Removed |Added ---------------------------------------------------------------------------- OtherBugsDependingO|163778, 177841 |163779 nThis| |
------- Additional Comments From toshio@tiki-lounge.com 2006-09-15 17:54 EST ------- Congratulations!
APPROVING
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: dstat
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=199780
scott@perturb.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |CLOSED Resolution| |CURRENTRELEASE Fixed In Version| |0.6.3-5
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: dstat
https://bugzilla.redhat.com/show_bug.cgi?id=199780
bugzilla@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Severity|normal |medium Priority|normal |medium Product|Fedora Extras |Fedora Version|devel |rawhide
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=199780
Dan Horák dan@danny.cz changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |adrianalves@fedoraproject.o | |rg
--- Comment #21 from Dan Horák dan@danny.cz 2008-09-12 01:28:43 EDT --- *** Bug 462034 has been marked as a duplicate of this bug. ***
package-review@lists.fedoraproject.org