Bug 482884
Summary: | Review Request: clc-intercal - Compiler for the INTERCAL language | ||
---|---|---|---|
Product: | [Fedora] Fedora | Reporter: | Iain Arnell <iarnell> |
Component: | Package Review | Assignee: | Jason Tibbitts <j> |
Status: | CLOSED NEXTRELEASE | QA Contact: | Fedora Extras Quality Assurance <extras-qa> |
Severity: | medium | Docs Contact: | |
Priority: | medium | ||
Version: | rawhide | CC: | fedora-package-review, jochen, notting |
Target Milestone: | --- | Flags: | j:
fedora-review+
kevin: fedora-cvs+ |
Target Release: | --- | ||
Hardware: | All | ||
OS: | Linux | ||
Whiteboard: | |||
Fixed In Version: | 0-0.1.1._94._2.fc9 | Doc Type: | Bug Fix |
Doc Text: | Story Points: | --- | |
Clone Of: | Environment: | ||
Last Closed: | 2009-03-27 14:49:31 UTC | Type: | --- |
Regression: | --- | Mount Type: | --- |
Documentation: | --- | CRM: | |
Verified Versions: | Category: | --- | |
oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |
Cloudforms Team: | --- | Target Upstream Version: | |
Embargoed: |
Description
Iain Arnell
2009-01-28 17:22:35 UTC
Question: It this your first Fedora package? I ask this, because the package look like it's need some love. I try to summarize my first mind about this package: 1. At first, you should try to get a better description the package, so a user which want to install the package may got a idea, why he should try out this new programming language 2. Your have specified 0 as the global package version and add version information on the definition of subpackage. This is not ok, you should put the version information on the Version statement and the subpackage shouldn't have any version information in thier name. 3.) It's look very ugly, that you are using rpm macros for the whole content of the Summary and Group statement. Unfortunately, I either a sponsor or a perl packaging specialist, so I can't review your package, but I would show your some points which are needs some homework. If this you first package and you need a sponsor please set the FE-NEEDSPONSOR bug as a blocker. Best Regards: Jochen Schmitt Answer: No, I already maintain several other perl packages. No sponsor required. 1. Okay, description was a little weak. New text ripped from Debian package: Summary: Compiler for the INTERCAL language Description: This package provides a Perl-based compiler for the INTERCAL programming language, usable either from the command line or as a Perl module. CLC-INTERCAL is designed to be almost compatible with the original (Princeton 1972) compiler. It also implements several extensions to the original language, including support for object orientation, operator overloading and quantum computing. 2. Indeed. But RPM can't cope with hyphens in version numbers (and even if it could, it probably wouldn't realise that 1.-94.-2 is a higher version number than 1.-94.-3). It seemed appropriate for INTERCAL to seek a particularly obtuse solution, but on second thought... Since it's not yet a 1.0 release, how about Version: 0 and Release: 0.%{X}.1._94._2 (as a non-numeric pre-release tag). 3. %{summary} comment is moot if I stick the proper version into release tag; the offending sub-package is no longer necessary. Only Group: %{group} remains in UI-X sub-package. New Spec: http://iarnell.fedorapeople.org/rpms/CLC-INTERCAL.spec New SRPM: http://iarnell.fedorapeople.org/rpms/CLC-INTERCAL-0-0.0.1._94._2.fc11.src.rpm New Build: http://koji.fedoraproject.org/koji/taskinfo?taskID=1090063 This is so horrible. And perverse. Which makes the package absolutely essential. It builds and installs fine for me. I haven't the slightest idea how to make it do anything, and frankly I don't want to learn for fear of mental contamination. Some comments: There's no need at all for the Group: tag in the subpackage, unless you want it to be different from the main package. In this case it's just pointless. I would urge the lower-casing of the package name, as Debian seems to do. I have some concerns about /usr/bin/sick as being insufficiently unique. A search turns up no instances anywhere except in the Debian package, so it seems low risk, but I wonder if it's worth it for what's essentially a joke package. I'm going to leave that up to you. You can read http://fedoraproject.org/wiki/Common_package_names_packaging_guideline_draft for guidance; it's still being drafted, but should grow to encompass potentially conflicting executables as well. I tried parallel make but the package then fails to build. Please add a comment to this effect so that folks looking to save a bit of build time won't be tripped up as I was. It's not immediately clear how the UI-X subpackage provides a graphical interface. I guess the other executables look for the presence of the X module and use it if present. Is it reasonable to provide a desktop file and an icon in that case? * source files match upstream. sha256sum: 6f3db1dc35c9217c6590007238d444e30730ee021b4ea4338b26bdb7b8ff29ac CLC-INTERCAL-1.-94.-2.tar.gz * package meets naming and versioning guidelines (as much as is possible) I suggest downcasing the package name. * specfile is properly named, is cleanly written and uses macros consistently. * summary is OK. * description is OK. * dist tag is present. * build root is OK. * license field matches the actual license. * license is open source-compatible. * license text included in package. * latest version is being packaged. * BuildRequires are proper. * %clean is present. * package builds in mock (rawhide, x86_64). * package installs properly. * rpmlint is silent. * final provides and requires are sane: CLC-INTERCAL-0-0.0.1._94._2.fc11.noarch.rpm perl(Language::INTERCAL::ArrayIO) perl(Language::INTERCAL::Arrays) perl(Language::INTERCAL::Arrays::Hybrid) perl(Language::INTERCAL::Arrays::Tail) perl(Language::INTERCAL::Backend) perl(Language::INTERCAL::Backend::ListObject) perl(Language::INTERCAL::Backend::Object) perl(Language::INTERCAL::Backend::Perl) perl(Language::INTERCAL::Backend::Run) perl(Language::INTERCAL::ByteCode) perl(Language::INTERCAL::Charset) perl(Language::INTERCAL::Charset::Baudot) perl(Language::INTERCAL::Charset::EBCDIC) perl(Language::INTERCAL::Charset::Hollerith) perl(Language::INTERCAL::CrawlingHorror) perl(Language::INTERCAL::DataItem) perl(Language::INTERCAL::Distribute) perl(Language::INTERCAL::DoubleOhSeven) perl(Language::INTERCAL::Exporter) perl(Language::INTERCAL::Generate) perl(Language::INTERCAL::GenericIO) perl(Language::INTERCAL::GenericIO::ARRAY) perl(Language::INTERCAL::GenericIO::COUNT) perl(Language::INTERCAL::GenericIO::FILE) perl(Language::INTERCAL::GenericIO::OBJECT) perl(Language::INTERCAL::GenericIO::REMOTE) perl(Language::INTERCAL::GenericIO::STRING) perl(Language::INTERCAL::GenericIO::TCP) perl(Language::INTERCAL::GenericIO::TEE) perl(Language::INTERCAL::GenericIO::UFILE) perl(Language::INTERCAL::GenericIO::UTCP) perl(Language::INTERCAL::HostIP) perl(Language::INTERCAL::InstallModule) perl(Language::INTERCAL::Interface) perl(Language::INTERCAL::Interface::Curses) perl(Language::INTERCAL::Interface::Line) perl(Language::INTERCAL::Interface::Line::IN) perl(Language::INTERCAL::Interface::Line::WOBJ) perl(Language::INTERCAL::Interface::None) perl(Language::INTERCAL::Interface::common) perl(Language::INTERCAL::Interpreter) perl(Language::INTERCAL::Numbers) perl(Language::INTERCAL::Numbers::Spot) perl(Language::INTERCAL::Numbers::Twospot) perl(Language::INTERCAL::Object) perl(Language::INTERCAL::Optimiser) perl(Language::INTERCAL::Parser) perl(Language::INTERCAL::Rcfile) perl(Language::INTERCAL::ReadNumbers) perl(Language::INTERCAL::Reggrim) perl(Language::INTERCAL::Server) perl(Language::INTERCAL::SharkFin) perl(Language::INTERCAL::Sick) perl(Language::INTERCAL::Splats) perl(Language::INTERCAL::SymbolTable) perl(Language::INTERCAL::Theft) perl(Language::INTERCAL::Whirlpool) perl(Language::INTERCAL::WriteNumbers) CLC-INTERCAL = 0-0.0.1._94._2.fc11 = /usr/bin/perl perl >= 0:5.005 perl(Carp) perl(Config) perl(Curses) perl(Exporter) perl(ExtUtils::MakeMaker) perl(File::Basename) perl(File::Spec) perl(File::Spec::Functions) perl(FindBin) perl(Getopt::Long) perl(IO::File) perl(IO::Socket::INET) perl(Language::INTERCAL::ArrayIO) perl(Language::INTERCAL::Arrays) perl(Language::INTERCAL::Backend) perl(Language::INTERCAL::Backend::Object) perl(Language::INTERCAL::ByteCode) perl(Language::INTERCAL::Charset) perl(Language::INTERCAL::Charset::Baudot) perl(Language::INTERCAL::CrawlingHorror) perl(Language::INTERCAL::DataItem) perl(Language::INTERCAL::DoubleOhSeven) perl(Language::INTERCAL::Exporter) perl(Language::INTERCAL::GenericIO) perl(Language::INTERCAL::GenericIO::FILE) perl(Language::INTERCAL::GenericIO::TCP) perl(Language::INTERCAL::HostIP) perl(Language::INTERCAL::Interface) perl(Language::INTERCAL::Interface::common) perl(Language::INTERCAL::Interpreter) perl(Language::INTERCAL::Numbers) perl(Language::INTERCAL::Object) perl(Language::INTERCAL::Optimiser) perl(Language::INTERCAL::Parser) perl(Language::INTERCAL::Rcfile) perl(Language::INTERCAL::ReadNumbers) perl(Language::INTERCAL::Reggrim) perl(Language::INTERCAL::Server) perl(Language::INTERCAL::SharkFin) perl(Language::INTERCAL::Sick) perl(Language::INTERCAL::Splats) perl(Language::INTERCAL::SymbolTable) perl(Language::INTERCAL::Whirlpool) perl(POSIX) perl(Socket) perl(Term::ReadLine) perl(constant) perl(strict) perl(vars) CLC-INTERCAL-UI-X-0-0.0.1._94._2.fc11.noarch.rpm perl(Language::INTERCAL::Interface::X) CLC-INTERCAL-UI-X = 0-0.0.1._94._2.fc11 = CLC-INTERCAL = 0-0.0.1._94._2.fc11 perl(:MODULE_COMPAT_5.10.0) perl(Carp) perl(Gtk2) perl(Language::INTERCAL::Exporter) perl(Language::INTERCAL::Interface::common) perl(strict) perl(vars) * %check is present and all tests pass: All tests successful. Files=15, Tests=6131, 17 wallclock secs ( 0.96 usr 0.10 sys + 15.77 cusr 0.77 csys = 17.60 CPU) All tests successful. Files=7, Tests=148, 7 wallclock secs ( 0.05 usr 0.02 sys + 0.36 cusr 0.06 csys = 0.49 CPU) All tests successful. Files=3, Tests=12, 0 wallclock secs ( 0.01 usr 0.02 sys + 0.20 cusr 0.02 csys = 0.25 CPU) All tests successful. Files=1, Tests=3, 0 wallclock secs ( 0.01 usr 0.00 sys + 0.12 cusr 0.01 csys = 0.14 CPU) All tests successful. Files=1, Tests=3, 0 wallclock secs ( 0.01 usr 0.00 sys + 0.13 cusr 0.02 csys = 0.16 CPU) All tests successful. Files=1, Tests=3, 0 wallclock secs ( 0.01 usr 0.01 sys + 0.16 cusr 0.03 csys = 0.21 CPU) * owns the directories it creates. * doesn't own any directories it shouldn't. * no duplicates in %files. * file permissions are appropriate. ? possible generically named files (/usr/bin/sick) * code, not content. * documentation is small, so no -doc subpackage is necessary. * %docs are not necessary for the proper functioning of the package. ? Maybe there should be a desktop file and icon in the UI-X subpackage. The package review process needs reviewers! If you haven't done any package reviews recently, please consider doing one. Thanks for the review. And I will start to reciprocate now that I've got a few under my belt. I agree with lowercasing - better aesthetically and consistent with debian. I've kept the group tag in the sub-package since at least on my setup, rpm is making it 'unspecified' if I don't (and I think rpm 4.4 requires it, so may still be necessary for F-9?). I'm happy with /usr/bin/sick - it certainly seems to be unique at the minute and should a possible conflict occur in future, I don't see too many problems if we do need to rename it later (anyone relying on intercal has more serious problems to worry about). You're absolutely right about the UI-X sub-package. I've added a sentence to its description to make this clearer. I'm not sure that a desktop file is necessary, though. New spec: http://iarnell.fedorapeople.org/rpms/clc-intercal.spec New SRPM: http://iarnell.fedorapeople.org/rpms/clc-intercal-0-0.1.1._94._2.fc11.src.rpm New koji: http://koji.fedoraproject.org/koji/taskinfo?taskID=1229460 You're right about the group tag; it seems that newer rpm versions handle this better, but older ones don't. I think we've touched on any potential issues, and everything significant has been addressed, so I think we're done. APPROVED New Package CVS Request ======================= Package Name: clc-intercal Short Description: Compiler for the INTERCAL language Owners: iarnell Branches: F-9 F-10 InitialCC: cvs done. clc-intercal-0-0.1.1._94._2.fc10 has been submitted as an update for Fedora 10. http://admin.fedoraproject.org/updates/clc-intercal-0-0.1.1._94._2.fc10 clc-intercal-0-0.1.1._94._2.fc9 has been submitted as an update for Fedora 9. http://admin.fedoraproject.org/updates/clc-intercal-0-0.1.1._94._2.fc9 clc-intercal-0-0.1.1._94._2.fc10 has been pushed to the Fedora 10 testing repository. If problems still persist, please make note of it in this bug report. If you want to test the update, you can install it with su -c 'yum --enablerepo=updates-testing update clc-intercal'. You can provide feedback for this update here: http://admin.fedoraproject.org/updates/F10/FEDORA-2009-2613 clc-intercal-0-0.1.1._94._2.fc9 has been pushed to the Fedora 9 testing repository. If problems still persist, please make note of it in this bug report. If you want to test the update, you can install it with su -c 'yum --enablerepo=updates-testing-newkey update clc-intercal'. You can provide feedback for this update here: http://admin.fedoraproject.org/updates/F9/FEDORA-2009-2622 clc-intercal-0-0.1.1._94._2.fc10 has been pushed to the Fedora 10 stable repository. If problems still persist, please make note of it in this bug report. clc-intercal-0-0.1.1._94._2.fc9 has been pushed to the Fedora 9 stable repository. If problems still persist, please make note of it in this bug report. |