Spec URL: http://www.knox.net.nz/~michael/buildbot.spec SRPM URL: http://www.knox.net.nz/~michael/buildbot-0.7.3-1.src.rpm Description: The BuildBot is a system to automate the compile/test cycle required by most software projects to validate code changes. By automatically rebuilding and testing the tree each time something has changed, build problems are pinpointed quickly, before other developers are inconvenienced by the failure.
See bug 197608, one of these should be closed as a dupe.
A quick review for your package, based on differences from what I used in mine. 1. Group is valid, but I preferred Development/Build Tools 2. Source0: can't find it right now, but I read somewhere the download location from sourceforge pacakges should be set to http://download.sourceforge.net/ and not on of the mirrors 3. the require on python-abi seems to be redundant on FC4 onwards. If you are _not_ going to build this also for FC3, you can safely remove the Req and the second line on the spec. 4. I used Requires: python-twisted >= 1.3.0 (from the buildbot web site); it's probably safe to omit that unless some repo has an older version. 5. Requires: python-cvstoys is not _really_ required. I know we lack a "Enhances" tag, so that's your choice. I am generally against pushing in packages if not strictly necessary. 6. I think it's better to run %{__pyhton} instead of hardcoding python 7. if you feel comfortable with maintaining the full list of files, leave it that way. Otherwise, the I found the solution drafted at http://fedoraproject.org/wiki/Packaging/Python being quite smart None of this points seems to be blockers, but I hope you can consider them carefully.
Thanks for the review. 1) used I changed it to Development/Tools as Development/Build Tools is not a valid group. 2) fixed 3) fixed 4) fixed 5) I am choosing to leave this one it. buildbot is 100x more functional with it. 6) fixed 7) its only a small files list, so in this case I will leave it. If it were bigger, then I would certainly use the method suggested on the wiki. Updated: Spec URL: http://www.knox.net.nz/~michael/buildbot.spec SRPM URL: http://www.knox.net.nz/~michael/buildbot-0.7.3-2.src.rpm
(In reply to comment #3) > 1) used I changed it to Development/Tools as Development/Build Tools is not a > valid group. That's good, but AFAIK "Development/Build Tools" _is_ a valid group. At least it is listed in http://fedoraproject.org/wiki/RPMGroups > 5) I am choosing to leave this one it. buildbot is 100x more functional with it. OK > 7) its only a small files list, so in this case I will leave it. If it were > bigger, then I would certainly use the method suggested on the wiki. Good I can't see any other blockers. rpmlint is silent on the srpm while on the actual rpm gives 3 warnings I would ignore: W: buildbot wrong-file-end-of-line-encoding /usr/share/doc/buildbot-0.7.3/contrib/windows/buildbot2.bat W: buildbot doc-file-dependency /usr/share/doc/buildbot-0.7.3/contrib/run_maxq.py /usr/bin/env W: buildbot doc-file-dependency /usr/share/doc/buildbot-0.7.3/contrib/svn_buildbot.py /usr/bin/env
*** Bug 197608 has been marked as a duplicate of this bug. ***
(In reply to comment #4) > That's good, but AFAIK "Development/Build Tools" _is_ a valid group. At least > it is listed in http://fedoraproject.org/wiki/RPMGroups I think that page is somewhat misleading and its intention is not to say that all values for the group tag found in FC4 (the latter list, where Development/Build Tools is) are ok. /usr/share/doc/rpm-*/GROUPS is the list of group tag values generally considered valid.
Thanks for the info bout valid groups. So, I can't see anything else preventing this from being committed. I hope someone will accept it soon (I'm deplyoing a buildbot based infrastructure at work).
Hey Gianluca, you weren't doing the review?
It doesn't like Gianluca has the access necessary for doing reviews; I'll try to take a look at this today or tomorrow if I can find the time and if nobody gets to it before me. From reading the above comments (as I haven't built the package), the rpmlint warnings stand out: W: buildbot wrong-file-end-of-line-encoding /usr/share/doc/buildbot-0.7.3/contrib/windows/buildbot2.bat What's the point of including the windows bits at all? W: buildbot doc-file-dependency /usr/share/doc/buildbot-0.7.3/contrib/run_maxq.py /usr/bin/env W: buildbot doc-file-dependency /usr/share/doc/buildbot-0.7.3/contrib/svn_buildbot.py /usr/bin/env Documentation shouldn't be executable.
Hi Jason. I left the Windows batch script there, largely because I work with buildbot in a mixed eviroment. It was useful to me when I initially got buildbot working on Windows. I figured it might be useful to others. Is no big deal, I can nuke it. Will fix the permissions on the contribs. Thanks
(In reply to comment #9) > It doesn't like Gianluca has the access necessary for doing reviews; Sorry for lurking on this. Rex is true: AFAIK official reviews leading to ACCEPTED status are meant to be made from any current package owner or SPONSORS (in case this is your first package). Unfortunately, I do not belong to either group :( > > From reading the above comments (as I haven't built the package), the rpmlint > warnings stand out: > > W: buildbot wrong-file-end-of-line-encoding > /usr/share/doc/buildbot-0.7.3/contrib/windows/buildbot2.bat > > What's the point of including the windows bits at all? > > W: buildbot doc-file-dependency > /usr/share/doc/buildbot-0.7.3/contrib/run_maxq.py /usr/bin/env > W: buildbot doc-file-dependency > /usr/share/doc/buildbot-0.7.3/contrib/svn_buildbot.py /usr/bin/env > > Documentation shouldn't be executable. maybe the whole "contrib" directory can go in /usr/share/buildbot (along the line of /usr/share/cvs/contrib/), since those are support scripts and not really documentation. One last remark: I can not recall where I read it, but I believe it is recommended to not repeat the name of the package in the Summary field.
oops... s/Rex/Jason :)
Updated: Spec URL: http://www.knox.net.nz/~michael/buildbot.spec SRPM URL: http://www.knox.net.nz/~michael/buildbot-0.7.3-3.src.rpm Moved the contribs to /usr/share/buildbot/contribs Thanks for that suggestion!
Well, the python guidelines have been changed to get rid of the "ghost the .pyo files" bit. So your files section should shrink a bit. (In any case, you didn't ghost the .pyo files in the contrib directory.) This builds fine in mock; rpmlint has the following to say about the SRPM: W: buildbot mixed-use-of-spaces-and-tabs You use spaces everywhere except for Patch0: and BuildArch:. Not a really big deal. There are many subdirectories under python_sitelyb/buildbot that you don't own. Getting rid of the %ghost bits should fix this as well. There's something that looks like a test suite in buildbot/test. Is this something that could be run at package build time? Should it really be included with the installed package? Review: * source files match upstream: 7be16fe13f173e46df711ed51648e750 buildbot-0.7.3.tar.gz * package meets naming and packaging guidelines. * specfile is properly named, is cleanly written and uses macros consistently. * dist tag is present. * build root is correct. * license field matches the actual license. * license is open source-compatible. License text not included upstream. * latest version is being packaged. * BuildRequires are proper. * %clean is present. * package builds in mock (development, x86_64). X rpmlint is silent. * final provides and requires are sane: buildbot = 0.7.3-3.fc6 = /usr/bin/env /usr/bin/python python(abi) = 2.4 python-cvstoys python-twisted >= 1.3.0 ? %check is not present but there might be a test suite. * package is not relocatable. X owns the directories it creates. * doesn't own any directories it shouldn't. * no duplicates in %files. * file permissions are appropriate. * no scriptlets present. * code, not content. * documentation is small, so no -docs subpackage is necessary. * %docs are not necessary for the proper functioning of the package. * no headers. * no pkgconfig files. * no libtool .la droppings.
Hey, just a quick ping to let you know that I am alive, just still in the trows of unpacking/new job/etc etc. I hope to tidy this review up before/by the end of the week. Thanks for your patience.
0.7.4 is out
Updated: Spec URL: http://www.knox.net.nz/~michael/buildbot.spec SRPM URL: http://www.knox.net.nz/~michael/buildbot-0.7.4-1.src.rpm
You still have several unowned directories under %{python_sitelib}/buildbot. Why can't you just have a %files section like: %files %defattr(-,root,root,-) %doc ChangeLog NEWS README docs %{_bindir}/buildbot %{python_sitelib}/buildbot %{_datadir}/%{name} instead of all of those globs. Also, could you comment on the test suite issue I brought up in comment 14?
Sorry. Missed that. The test stuff is for running tests on a source tree (if availible), like "make test". Its part of buildbot's process. Its not a test for buildbot itself. Will wrap up a new srpm shortly.
Updated: Spec URL: http://www.knox.net.nz/~michael/buildbot.spec SRPM URL: http://www.knox.net.nz/~michael/buildbot-0.7.4-2.src.rpm
OK, everything looks fine now. THe directories all look to be properly owned, rpmlint is silent and what I thought might be a test suite isn't. APPROVED
Thanks! imported and built.
I am going to unorphan this package; please replace old owner.
Package Change Request ====================== Package Name: buildbot New Branches: EL-5 Owners: smilner giallu
cvs done.
Package Change Request ====================== Package Name: buildbot New Branches: el6 Owners: smilner giallu Steve, I hope it is ok for you to co-maintain also the el6 branch, if not just let me know
Git done (by process-git-requests).
Package Change Request ====================== Package Name: buildbot New Branches: epel7 Owners: besser82 smilner giallu radez We need a build for EPEL7… ^^
Is anybody working on EPEL7 build?