[Bug 565251] New: Review Request: coan - A commandline tool for simplifying the preprocessor conditionals in source code
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
Summary: Review Request: coan - A commandline tool for simplifying the preprocessor conditionals in source code
https://bugzilla.redhat.com/show_bug.cgi?id=565251
Summary: Review Request: coan - A commandline tool for simplifying the preprocessor conditionals in source code Product: Fedora Version: rawhide Platform: All OS/Version: Linux Status: NEW Severity: medium Priority: medium Component: Package Review AssignedTo: nobody@fedoraproject.org ReportedBy: eric@brouhaha.com QAContact: extras-qa@fedoraproject.org CC: notting@redhat.com, fedora-package-review@redhat.com Estimated Hours: 0.0 Classification: Fedora
Spec URL: http://fedorapeople.org/~brouhaha/coan/coan.spec SRPM URL: http://fedorapeople.org/~brouhaha/coan/coan-4.0-1.fc12.src.rpm Koji scratch build for F-12: http://koji.fedoraproject.org/koji/taskinfo?taskID=1984936 Description:
Coan is a software engineering tool for analysing preprocessor-based configurations of C or C++ source code. Its principle use is to simplify a body of source code by eliminating any parts that are redundant with respect to a specified configuration. It is a more powerful successor to the FreeBSD 'unifdef' tool and the 'sunifdef' tool.
Coan is most useful to developers of constantly evolving products with large code bases, where preprocessor conditionals and #if-directives are used to differentiate progressive releases or parallel variants of the product. In these settings the upkeep of the product's configuration tree can become difficult and the incidence of configuration-related defects can become costly. Coan can largely automate the maintenance of preprocessor-based configurations and the investigation of configuration- related questions.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=565251
Mike Kinghan imk@abstract-art.co.uk changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |imk@abstract-art.co.uk
--- Comment #1 from Mike Kinghan imk@abstract-art.co.uk 2010-02-24 03:28:58 EST --- Hi,
I'm the author of coan. coan is the tool formerly known as sunifdef. For v4.0, I renamed and relaunched it because it had out-grown its original "son-of-unifdef" remit. sunifdef has been a fedora extras package since 2006. If you take up coan for fedora extras, I guess you should drop sunifdef.
Mike
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=565251
--- Comment #2 from Mike Kinghan imk@abstract-art.co.uk 2010-04-05 11:39:46 EDT --- Coan is now released at v4.1 with fixes/features here http://coan2.sourceforge.net/index.php?page=changes
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=565251
--- Comment #3 from Eric Smith eric@brouhaha.com 2010-04-06 19:38:52 EDT --- Thanks, Mike! I've updated the package for review:
Spec URL: http://fedorapeople.org/~brouhaha/coan/coan.spec SRPM URL: http://fedorapeople.org/~brouhaha/coan/coan-4.1-1.fc12.src.rpm Koji scratch build for F-13: http://koji.fedoraproject.org/koji/taskinfo?taskID=2098565
Because coan seems to have substantial differences in the command-line interface, I don't think the coan package should obsolete the sunifdef package. People may have scripts using sunifdef that would break.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=565251
--- Comment #4 from Mike Kinghan imk@abstract-art.co.uk 2010-04-07 05:00:21 EDT --- That's fair enough.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=565251
Ankur Sinha sanjay.ankur@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED CC| |sanjay.ankur@gmail.com AssignedTo|nobody@fedoraproject.org |sanjay.ankur@gmail.com Flag| |fedora-review?
--- Comment #5 from Ankur Sinha sanjay.ankur@gmail.com 2010-05-07 00:53:04 EDT --- hey,
looks simple enough.
A quick rpmlint:
coan.src: W: spelling-error %description -l en_US analysing -> analyzing, analysis, analysand
I'll do a detailed review hopefully by the weekend.
regards, Ankur
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=565251
--- Comment #6 from Ankur Sinha sanjay.ankur@gmail.com 2010-05-14 15:36:07 EDT --- Review:
+ - OK - - NA ? - ISSUE
+++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
+ Package meets naming and packaging guidelines + Spec file matches base package name. + Spec has consistant macro usage. + Meets Packaging Guidelines. ? License + License field in spec matches + License file included in package + Spec in American English + Spec is legible. + Sources match upstream md5sum:
[Ankur@localhost SPECS]$ md5sum coan-4.1.tar.gz 33ccc73af33c4162ecb55ab1816cd2ae coan-4.1.tar.gz [Ankur@localhost SPECS]$ md5sum ../SOURCES/coan-4.1.tar.gz 33ccc73af33c4162ecb55ab1816cd2ae ../SOURCES/coan-4.1.tar.gz
- Package needs ExcludeArch + BuildRequires correct - Spec handles locales/find_lang - Package is relocatable and has a reason to be. + Package has %defattr and permissions on files is good. + Package has a correct %clean section. + Package has correct buildroot %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n) + Package is code or permissible content. - Doc subpackage needed/used. + Packages %doc files don't affect runtime.
- Headers/static libs in -devel subpackage. - Spec has needed ldconfig in post and postun - .pc files in -devel subpackage/requires pkgconfig - .so files in -devel subpackage. - -devel package Requires: %{name} = %{version}-%{release} - .la files are removed.
- Package is a GUI app and has a .desktop file
+ Package compiles and builds on at least one arch. + Package has no duplicate files in %files. + Package doesn't own any directories other packages own. + Package owns all the directories it creates. ? No rpmlint output.
- final provides and requires are sane: (include output of for i in *rpm; do echo $i; rpm -qp --provides $i; echo =; rpm -qp --requires $i; echo; done
[Ankur@localhost x86_64]$ for i in *rpm; do echo $i; rpm -qp --provides $i; echo =; rpm -qp --requires $i; echo; done coan-4.1-1.fc13.x86_64.rpm coan = 4.1-1.fc13 coan(x86-64) = 4.1-1.fc13 ..
coan-debuginfo-4.1-1.fc13.x86_64.rpm coan-debuginfo = 4.1-1.fc13 coan-debuginfo(x86-64) = 4.1-1.fc13 ..
SHOULD Items:
+ Should build in mock. + Should build on all supported archs
- Should function as described. not checked yet
- Should have sane scriptlets. - Should have subpackages require base package with fully versioned depend. + Should have dist tag + Should package latest version - check for outstanding bugs on package. (For core merge reviews)
Issues:
1.rpmlint output
[Ankur@localhost SPECS]$ rpmlint coan.spec ../RPMS/x86_64/coan-* ../SRPMS/coan-4.1-1.fc13.src.rpm coan.x86_64: W: spelling-error Summary(en_US) commandline -> command line, command-line, commandment coan.x86_64: W: spelling-error Summary(en_US) preprocessor -> processor, teleprocessing, processional coan.x86_64: W: spelling-error %description -l en_US analysing -> analyzing, analysis, analysand coan.x86_64: W: spelling-error %description -l en_US preprocessor -> processor, teleprocessing, processional coan.x86_64: W: spelling-error %description -l en_US unifdef -> uniform, unify, Unicode coan.x86_64: W: spelling-error %description -l en_US sunifdef -> sundries, sunshade, Sunnite
coan.x86_64: W: invalid-url URL: http://coan2.sourceforge.net/ <urlopen error [Errno -2] Name or service not known> coan.src: W: spelling-error Summary(en_US) commandline -> command line, command-line, commandment coan.src: W: spelling-error Summary(en_US) preprocessor -> processor, teleprocessing, processional coan.src: W: spelling-error %description -l en_US analysing -> analyzing, analysis, analysand coan.src: W: spelling-error %description -l en_US preprocessor -> processor, teleprocessing, processional coan.src: W: spelling-error %description -l en_US unifdef -> uniform, unify, Unicode coan.src: W: spelling-error %description -l en_US sunifdef -> sundries, sunshade, Sunnite coan.src: W: invalid-url URL: http://coan2.sourceforge.net/ <urlopen error [Errno -2] Name or service not known> 3 packages and 1 specfiles checked; 0 errors, 14 warnings.
Minor spelling errors,
2. the src directory has a COPYING file with GPLv3 in it. I don't see why its there. Please just confirm the license of the package. Since the source files say BSD, the file BSD is good.
The rest looks good. On its way to approval :)
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=565251
--- Comment #7 from Eric Smith eric@brouhaha.com 2010-05-17 18:58:59 EDT --- I just emailed the author to get a clarification with regard to the license and that COPYING file.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=565251
--- Comment #8 from Eric Smith eric@brouhaha.com 2010-05-21 20:49:18 EDT --- Mike confirms that there is not actually anything in the coan package covered by the GPL. It's all BSD. Apparently the COPYING file in question shouldn't be there, and he's going to remove it. I'll update my spec and SRPM when he's got a new release ready.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=565251
--- Comment #9 from Eric Smith eric@brouhaha.com 2010-05-28 16:15:43 EDT --- License file issue now tracked on SourceForge: https://sourceforge.net/tracker/?func=detail&aid=3008737&group_id=25...
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=565251
--- Comment #10 from Eric Smith eric@brouhaha.com 2010-05-31 19:28:36 EDT --- The license file issue was fixed upstream by removing the COPYING file and other unnecessary files from the src directory. Note that the upstream version number was NOT changed, so the tarball has the same name but different contents and hashes.
Spec URL: http://fedorapeople.org/~brouhaha/coan/coan.spec SRPM URL: http://fedorapeople.org/~brouhaha/coan/coan-4.1-2.fc12.src.rpm Koji scratch build for F-13: http://koji.fedoraproject.org/koji/taskinfo?taskID=2221010
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=565251
Ankur Sinha sanjay.ankur@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flag|fedora-review? |fedora-review+
--- Comment #11 from Ankur Sinha sanjay.ankur@gmail.com 2010-07-09 03:36:48 EDT --- Looks good. Issues have been corrected.
Builds on F13, rpmlint gives minor warnings (spelling suggestions that can be ignored):
[Ankur@localhost SPECS]$ rpmlint ../RPMS/x86_64/coan-* coan.spec ../SRPMS/coan-4.1-2.fc13.src.rpm coan.x86_64: W: spelling-error Summary(en_US) commandline -> command line, command-line, commanding coan.x86_64: W: spelling-error Summary(en_US) preprocessor -> preprocessed, processor, preprofessional coan.x86_64: W: spelling-error %description -l en_US analysing -> analyzing, analysis, analysand coan.x86_64: W: spelling-error %description -l en_US preprocessor -> preprocessed, processor, preprofessional coan.x86_64: W: spelling-error %description -l en_US unifdef -> unifier, unifiable, uniform coan.x86_64: W: spelling-error %description -l en_US sunifdef -> sundries, sunshade, sunniness coan.x86_64: W: spelling-error Summary(en_US) preprocessor -> preprocessed, processor, preprofessional coan.x86_64: W: spelling-error %description -l en_US preprocessor -> preprocessed, processor, preprofessional coan.x86_64: W: spelling-error %description -l en_US unifdef -> unifier, unifiable, uniform coan.x86_64: W: spelling-error %description -l en_US sunifdef -> sundries, sunshade, sunniness coan.spec: W: invalid-url Source0: http://downloads.sourceforge.net/coan2/v4.1/coan-4.1.tar.gz <urlopen error timed out> coan.src: W: spelling-error Summary(en_US) preprocessor -> preprocessed, processor, preprofessional coan.src: W: spelling-error %description -l en_US preprocessor -> preprocessed, processor, preprofessional coan.src: W: spelling-error %description -l en_US unifdef -> unifier, unifiable, uniform coan.src: W: spelling-error %description -l en_US sunifdef -> sundries, sunshade, sunniness coan.src: W: invalid-url Source0: http://downloads.sourceforge.net/coan2/v4.1/coan-4.1.tar.gz <urlopen error timed out> 5 packages and 1 specfiles checked; 0 errors, 16 warnings.
Do change "commandline" to "command line" when you push it to the repos though.
XXX APPROVED XXX
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=565251
--- Comment #12 from Eric Smith eric@brouhaha.com 2010-09-09 23:14:32 EDT --- Just noticed that you'd approved this; I don't seem to have gotten the email that Bugzilla automatically generated. The "commandline" to "command line" change was actually already in the spec you reviewed; it looks like you still had a previously built RPM in your RPMS/x86_64 directory when you ran rpmlint.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=565251
Eric Smith eric@brouhaha.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flag| |fedora-cvs?
--- Comment #13 from Eric Smith eric@brouhaha.com 2010-09-09 23:18:53 EDT --- New Package SCM Request ======================= Package Name: coan Short Description: A command line tool for simplifying the preprocessor conditionals in source code Owners: brouhaha Branches: f12 f13 f14 InitialCC:
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=565251
--- Comment #14 from Kevin Fenzi kevin@tummy.com 2010-09-10 14:52:00 EDT --- Uh. This package seems to already be in fedora? See https://bugzilla.redhat.com/show_bug.cgi?id=603366
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=565251
Eric Smith eric@brouhaha.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |CLOSED Resolution| |NOTABUG Flag|fedora-cvs? | Last Closed| |2010-09-10 15:07:54
--- Comment #15 from Eric Smith eric@brouhaha.com 2010-09-10 15:07:54 EDT --- Well, it wasn't when I started this review, and I'm not sure that the right thing happened, as coan != sunifdef. There are incompatible differences, and the package rename may break some people's scripts. However, since it did happen, I'll withdraw the SCM request and close this bug.
package-review@lists.fedoraproject.org
-
Red Hat Bugzilla