[Bug 193894] Review Request: ant-contrib - A collection of tasks for Ant

[bugzilla at redhat.com]

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: ant-contrib - A collection of tasks for Ant


https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=193894


j.w.r.degoede at hhs.nl changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
         AssignedTo|bugzilla-sink at leemhuis.info |j.w.r.degoede at hhs.nl
                 CC|j.w.r.degoede at hhs.nl        |
OtherBugsDependingO|163776                      |163778
              nThis|                            |




------- Additional Comments From j.w.r.degoede at hhs.nl  2006-06-23 06:11 EST -------
Ok,

I'll start reviewing those packages then starting with this one and sorry for
being somewhat slow, I'm currently rather busy with work.

Ok, here we go this is the first java package I'm reviewing so feel free to have
a different opinion in certain cases:

MUST:
=====
* rpmlint output is:
W: ant-contrib non-standard-group Development/Libraries/Java
W: ant-contrib non-standard-group Development/Libraries/Java
W: ant-contrib class-path-in-manifest /usr/share/java/ant-contrib-1.0b2.jar
W: ant-contrib-javadoc non-standard-group Development/Documentation
W: ant-contrib-javadoc dangerous-command-in-%post rm
W: ant-contrib-manual non-standard-group Development/Documentation
W: ant-contrib-manual wrong-file-end-of-line-encoding
/usr/share/doc/ant-contrib-1.0b2/tasks/for.html
W: ant-contrib-manual wrong-file-end-of-line-encoding
/usr/share/doc/ant-contrib-1.0b2/tasks/foreach.html
These all must be fixed!
* Package and spec file named appropriately
* Packaged according to packaging guidelines
* License ok, license file included
* spec file is legible and in Am. English.
* Couldn't very if source matches upstream, sf.net gives a 500 internal serv 
  error.
* Compiles and builds on devel-x86_64
* BR: ok (see below)
* No locales
* No shared libraries
* Not relocatable
* Package owns / or requires all dirs (with some strangeness see Must fix below)
* No duplicate files & Permissions ok
* %clean & macro usage OK
* Contains code only
* %doc does not affect runtime, and isn't large enough to warrent a sub package!
* no -devel package needed, no libs / .la files.
* no gui -> no .desktop file required


MUST fix:
=========
* All rpmlint messages, see above
* Remove the unused section %define
* Remove "%define base_name ant-contrib", replace "Name: %{base_name}"
  with "Name: ant-contrib" and replace any uses of %base_name with %name
  I so no reason whatsoever for the existence and use of this macro accept
  obfuscation
* For indentation / lining up the list with Name, Version .... BuildRoot you
  use a mix of spaces and tabs and you seem to have your tabsize set to
  something else then 8. Please just spaces everywhere, the indentation is a
  mess know in my editor.
* Source1 isn't used anywhere, remove it
* Remove Epoch: 0, you should not explicitly set Epoch to 0.
* 1.0b2 contains alphanumeric, I don't know what the exact version scheme
  of upstream is, does b2 stands for beta 2, or was there first a 1.0 then a 
  1.0a then 1.0b, 1.0b1 and finally 1.0b2? Anyways unless upstreams creative
  numbering is as described above (1.0 -> 1.0a -> etc) it will break rpm's
  version comparison, please in that case use just 1.0 as version and and encode 
  the additioan b2 into the release tag as described here:
http://fedoraproject.org/wiki/Packaging/NamingGuidelines#head-e104844825856d7c45f2f0241586985c0495966b
  Also see the note about Release below under Should fix.
* Replace "%setup -q -c -n %{base_name}-%{version}" with
  "%setup -q -n %{name}" and remove all the pushd popd nonsense as that then
  no longer is nescesarry
* Remove the 2 find lines from %setup, the first is total nonsense and the 
  second one doesn't do anything either as there are no jar files included.
* Don't use cp to make manual backups of patched files (the 2 .sav files 
  created). Instead pass " -z .backupext" to the %patch commands
* For the manual subpackage you create %{_docdir}/%{name}-%{version}
  and then copy the docs there and next you put %{_docdir}/%{name}-%{version}
  under %files. This isn't nescesarry if you specify %doc a dir releative to
  %{builddir} (default /usr/src/redhat/BUILD/ant-contrib for this package) then
  it will create the dir, copy the files and at them to %files themselves.
  So:
  -drop the installing of these files from %install
  -under "%files manual" replace "%doc %{_docdir}/%{name}-%{version}" with
   "%doc build/docs/*". Since the license is already included and the index.html
   under build/docs/ contains install instructions, which we usually don't    
   package as the rpm does the installing for the user, I would even like to
   plea to change this too: "%doc build/docs/tasks/*"
* Don't put the manual in a seperate subpackage, its only 200k and people who
  really need the diskspace can tell rpm not to install anything marked %doc.
* whats this with this symlink  ghosting rm-ing black voodoo, why not just plain
  package the symlink, why is the symlink there at all?


Should fix:
===========
* We (Fedora) don't support building java packages using the JDK, I've checked a
  couple of other packages an no other has a gcj_support conditional. Please
  concider removing this and only leaving the gcj code in that will make things
  much easier to read.
* The Xjpp_Yfc Release fields in other packages are only used AFAIK to allow
  smooth upgrade from jpackage packages to Fedora ones, since you've upgraded
  to a newer upstream version upgrading from jpackage to FE should nnot be a 
  problem please use a regular 1%{?dist} release instead of 1jpp_1fc.
* Why the non standard %defattr(0644,root,root,0755) under %files why not just
  %defattr(-,root,root,-) ?
* Redundant BR (must ne removed): ant, alreayd implied by ant-junit.
* Are you sure it will only build with this very specific version of junit that
  looks like an error to me.


-- 
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.