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 ------- Additional Comments From gnome@xxxxxxxxxxxxx 2007-06-17 23:38 EST ------- General Review --> MUSTS: 1) RPMLINT: rpmlint produces no output - PASS 2) NAMING GUIDELINES: *no underscores in name (since the source contains underscores you could use underscores if you wanted, do you want to? If downstream contains them, which it does, it would be good to be consistent. ) - NEEDINFO *spec file name correct - PASS *package version matches source and correct - PASS *release number correct - PASS *%{?dist} used properly - PASS *package not case sensitive - PASS *no renaming/replacing existing packages - PASS *no subpackages included - NEEDINFO -- Cannot find astyle library files? -- Are the libs he referred to standard C++ includes? -- Are the libs source files in src/ ? -- It would probably be best to include the libs in another pkg. *no addon packages - PASS 3) PACKAGING GUIDELINES: *licensing incorrect - CHANGE -- astyle is licensed under the LGPL, not GPL -- please correct License: -- LGPL documentation included as license.html document *no known patents - NA *not an emulator - NA *no binary firmware - NA *inclusion of libs and/or binaries - NEEDINFO (see subpackages) *pkg from scratch matches minimal spec except %configure - NEEDINFO -- make %{?_smp_mflags} --> should be included if possible -- because the pkg is compiled manually maybe a g++ switch? -- g++ -o astyle $RPM_OPT_FLAGS src/*.cpp could be? - NOT SURE IF NEEDED It's a single (and small) source file so you shouldn't need it I'd think. -- I don't believe %configure is required - NEEDINFO *not modifying an existing spec - NA *filesystem layout - PASS *rpmlint - PASS *changelog - CHANGE -- should remove the last changelog comment about a different version -- 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 *buildroot - PASS *requires - *prereq - NA *file dependencies - NA *buildrequires - NEEDINFO -- mock produces the following during the debug ... cpio: astyle/<built-in>: No such file or directory -- I am not sure if the above "error" is relevant. -- I've attached the mock build log. -- builds cleanly in mock. *rpmdev-rmdevelrpms - NA *summary and description - PASS *encoding - PASS *non-ascii filenames - NA *documentation - PASS -- It would be nice to have a brief manpage for this pkg that the maintainers would incorporate. *compiler flags - UNKNOWN *debuginfo pkgs - PASS *static linked libraries/executables - NA *libraries - NEEDINFO -- I think this package might need to be split apart into a libraries pkg and a program pkg. I'm not totally sure though, since I'm somewhat "new." Please read up on packaging guidelines to determine the best way to get around the way the makefile is structured. (http://fedoraproject.org/wiki/Packaging/Guidelines) *duplication of sys libs - NA *rpath and removing rpath - NA *configuration file - NA *init scripts - NA *desktop file - NA *macros - PASS *locale files - NA *parallel make - NEEDINFO -- because this pkg uses g++ instead of make in the build section and doesn't have a configure section I don't know how to address this "nice to have." Anyone have an idea? *timestamps - PASS *scriptlets - NA *conditional dependencies - NA *relocatable pkg - NA *code vs. content - PASS *file and dir ownership - PASS *web app - NA *confilcts - NA 4) LICENSING: *need to correct the License: field - CHANGE *other than the problem above - PASS 5) LICENSE FIELD: *see #4 - CHANGE 6) DOC LICENSE: included in proper dir - PASS 7) SPEC FILE IN ENGLISH: - PASS 8) SPEC FILE LEGIBLE: - PASS 9) SOURCES MATCH UPSTREAM: need to replace last comment - PASS 10) COMPILES/BUILDS: built, and installed on my system (pkg runs) - PASS 11) EXCLUDEARCH: are there any arches it doesn't build for? I assume all are fine since the comments state any arch that has g++. - NEEDINFO 12) BUILDREQUIRES: none - PASS 13) LOCALES: none - PASS 14) SHARED LIBS: I think you should strip out the libs and put them in another pkg and change the makefile, but I'm new so a more experienced reviewer should chime in please... - NEEDINFO 15) RELOCATABLE PKG: n/a - NA 16) DIR OWNERSHIP: - PASS 17) DUPLICATE FILES: none - PASS 18) PERMS: - PASS 19) CLEAN SECTION: - PASS 20) CONSISTENT MACROS: - PASS 21) CODE/CONTENT: - PASS 22) LARGE DOCS: n/a - NA 23) MISSING DOCS: moved docs from docdir to home dir and binary functioned properly - PASS 24) HEADER FILES: there is a header in src but I don't think it needs to g into a -devel pkg - NEEDINFO (from a pkg reviewer) 25) STATIC LIBS: again, needinfo - NEEDINFO 26) PKGCONFIG: n/a - NA 27) LIBS w/ SUFFIXES: again, see #25 - NEEDINFO 28) DEVEL PKG: n/a - NA 29) LIBTOOL ARCHIVES: n/a - NA 30) GUI APP DESKTOP FILE: n/a - NA 31) OWN NON-SPECIFIC DIRS/FILES: no this pkg doesn't - PASS 32) REMOVE BUILDROOT IN INSTALL: - PASS 33) UTF-8 FILENAMES: - PASS SHOULDS: 1) Includes LGPL license text. - PASS 2) Description short and concise but no none English translations. This is up to the maintainer if they desire to have this included. - NEEDINFO 3) The package builds in mock with one potential issue: - NEEDINFO *mock produces the following during the debug stage ... cpio: astyle/<built-in>: No such file or directory *I'm not certain this is even an issue b/c the debuginfo pkgs build fine. 4) Package runs but I haven't tested the functionality further than 'astyle --help'. - PASS 5) Scriptlets are not used. - NA 6) No subpackages. There might be a lib pkg generated but again, that's a topic up for discussion with more experienced pkg reviewers. - NEEDINFO 7) Pkgconfig not used. - NA 8) No other known file dependencies other than the libs. - NEEDINFO -- 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