[Bug 207896] Review Request: astyle - Source code formatter
bugzilla at redhat.com
bugzilla at redhat.com
Mon Jun 18 03:38:57 UTC 2007
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 at dux-linux.org 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.
More information about the Fedora-package-review
mailing list