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: astyle - Source code formatter https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=207896 mtasaka@xxxxxxxxxxxxxxxxxxx changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |mtasaka@xxxxxxxxxxxxxxxxxxx ------- Additional Comments From mtasaka@xxxxxxxxxxxxxxxxxxx 2007-06-18 02:14 EST ------- (In reply to comment #12) > General Review --> Well, If you want to do the review of this style, please make the summary of the review so that everyone (including submitter) can read your review easily. > 2) NAMING GUIDELINES: > *no underscores in name (since the source contains underscores you could > use underscores if you wanted, do you want to? But the name "astyle" has no underscore.. I don't know what is the problem with the name of this pkgname? > *no subpackages included - NEEDINFO > -- It would probably be best to include the libs in another pkg. This can be left to how the submittter judges. > -- astyle is licensed under the LGPL, not GPL > *pkg from scratch matches minimal spec except %configure - NEEDINFO For this package this can be ignored IMO +1 for Ralf's comment > *rpmlint - PASS Umm??? Don't pass (see below) > *changelog - CHANGE > -- should remove the last changelog comment about a different version Why? > -- if another pkg exists with that version number then you should put > the comments in its' spec file > -- comment for initial version should match version of the pkg and > still exist so history of the pkg is maintained Currently I don't understand what you want to say here > -- mock produces the following during the debug ... > cpio: astyle/<built-in>: No such file or directory Can be ingored > *debuginfo pkgs - PASS Actually, don't pass (related with rpmlint - see below) > *libraries - NEEDINFO > -- I think this package might need to be split apart into a libraries > pkg and a program pkg. IMO this is not needed for this package. > *parallel make - NEEDINFO This is not needed for this package (g++ *.cpp meets the demand) > 4) LICENSING: > *need to correct the License: field - CHANGE * For rpmlint: ----------------------------------------------------------- E: astyle-debuginfo script-without-shebang /usr/src/debug/astyle/src/astyle_main.cpp W: astyle-debuginfo spurious-executable-perm /usr/src/debug/astyle/src/astyle.h E: astyle-debuginfo script-without-shebang /usr/src/debug/astyle/src/ASEnhancer.cpp E: astyle-debuginfo script-without-shebang /usr/src/debug/astyle/src/ASResource.cpp E: astyle-debuginfo script-without-shebang /usr/src/debug/astyle/src/ASFormatter.cpp E: astyle-debuginfo script-without-shebang /usr/src/debug/astyle/src/ASBeautifier.cpp ------------------------------------------------------------- - All of these are permission issues. Please fix these. * Macros - /usr/bin/install <- use macros for /usr/bin. * Documentation - Please check if "install.html" is needed for %doc. This seems to be needed for people who want to rebuild this package by themselves and does not seem to be needed for rpm users. -- Configure bugmail: https://bugzilla.redhat.com/bugzilla/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are the QA contact for the bug, or are watching the QA contact. _______________________________________________ Fedora-package-review mailing list Fedora-package-review@xxxxxxxxxx http://www.redhat.com/mailman/listinfo/fedora-package-review