[Bug 178142] Review Request: Jakarta Commons CLI

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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: Jakarta Commons CLI


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


chabotc@xxxxxxxxx changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
OtherBugsDependingO|163776                      |163778
              nThis|                            |




------- Additional Comments From chabotc@xxxxxxxxx  2006-01-18 06:22 EST -------
I see no one else picked this up in the meantime; Changing bug to FE-REVIEW

Looking at the spec file its still confusing to find the %define's, but i know
its to keep it close to the JPackage one, so thats ok :-)

Groups and everything look good from the get-go too

Summary: Its usually not needed to include the %{name} in it, rpm tools (or even
rpm -q) would display this name already before the summary, i think better would
be just:
"Command Line Interface for Java"

It builds and mock builds cleanly (fc-devel-i386)

rpmlint output is quiet.

It does have some files listed twice, rpmbuild says:
warning: File listed twice: /usr/lib/gcj/jakarta-commons-cli
warning: File listed twice:
/usr/lib/gcj/jakarta-commons-cli/jakarta-commons-cli-1.0.jar.db
warning: File listed twice:
/usr/lib/gcj/jakarta-commons-cli/jakarta-commons-cli-1.0.jar.so

It would be safe to make your files section:
%files                                                                         
                                             
%defattr(0644,root,root,0755)                                                  
                                             
%doc LICENSE.txt README.txt                                                    
                                             
%{_javadir}/*                                                                  
                                             
%{_libdir}/gcj/%{name}

That way it automaticly owns the directory, and picks up all the files inside of
it. File permissions look good to me, so no need for %attr magic

Formal review list:

MUST review items:
- Builds cleanly on FC5 devel.
- rpmlint has no output / complaints
- Source included matches upsteam source (md5sum)
- Package name meets guidelines
- spec file name is in %{name}.spec format
- Licence (Apache) is fedora extra's compatible & is included in spec
- Spec file is in (american) english
- Does not list buildrequires that are excepted in the package guidelines
- All build dependencies are listed
- No need for ldconfig
- All files have proper permissions
- Package is not relocatable
- ** Error: duplicate files in %files section
- No missing files in %files section
- Has a proper %clean section with rm -rf $RPM_BUILD_ROOT
- Uses macro's described in PackagingGuidelines
- No entries in %doc that are required for standard program operation
- No -devel package needed
- ** Directory-ownerships is ok, but needs rework to fix duplicate files
- Not a gui app so no desktop file handling needed

Should items:
- Includes upstream licence file (COPYING)
- No insane scriplets, or scriplets at all
- No unnescesarry requires
- Mock builds cleanly

If you could fix the 2 above mentioned issues (summary & %files section) i think
we'll be done with this in no time, nice to see your getting the hang of this
packaging thing :-)


-- 
Configure bugmail: https://bugzilla.redhat.com/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.

-- 
fedora-extras-list mailing list
fedora-extras-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/fedora-extras-list

[Index of Archives]     [Fedora General Discussion]     [Fedora Art]     [Fedora Docs]     [Fedora Package Review]     [Fedora Desktop]     [Big List of Linux Books]     [Yosemite Backpacking]     [KDE Users]

  Powered by Linux