[Bug 467235] Review Request: globus-callout - Globus Toolkit - Globus Callout Library

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


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


Orcan 'oget' Ogetbil <oget.fedora at gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
                 CC|                            |oget.fedora at gmail.com
         AssignedTo|nobody at fedoraproject.org    |oget.fedora at gmail.com
               Flag|                            |fedora-review?

Bug 467235 depends on bug 453847, which changed state.

Bug 453847 Summary: Review Request: grid-packaging-tools - The Grid Packaging Tools (GPT)
https://bugzilla.redhat.com/show_bug.cgi?id=453847

           What    |Old Value                   |New Value
----------------------------------------------------------------------------
             Status|ASSIGNED                    |CLOSED
         Resolution|                            |NEXTRELEASE

Bug 467235 depends on bug 453848, which changed state.

Bug 453848 Summary: Review Request: globus-core - Globus Toolkit - Globus Core
https://bugzilla.redhat.com/show_bug.cgi?id=453848

           What    |Old Value                   |New Value
----------------------------------------------------------------------------
         Resolution|                            |NEXTRELEASE
             Status|ASSIGNED                    |CLOSED

Bug 467235 depends on bug 453849, which changed state.

Bug 453849 Summary: Review Request: globus-libtool - Globus Toolkit - Globus libtool package (virtual GPT glue package)
https://bugzilla.redhat.com/show_bug.cgi?id=453849

           What    |Old Value                   |New Value
----------------------------------------------------------------------------
         Resolution|                            |NEXTRELEASE
             Status|ASSIGNED                    |CLOSED

Bug 467235 depends on bug 453851, which changed state.

Bug 453851 Summary: Review Request: globus-common - Globus Toolkit - Common Library
https://bugzilla.redhat.com/show_bug.cgi?id=453851

           What    |Old Value                   |New Value
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
         Resolution|                            |NEXTRELEASE
             Status|ASSIGNED                    |CLOSED



--- Comment #3 from Orcan 'oget' Ogetbil <oget.fedora at gmail.com>  2009-04-08 00:47:41 EDT ---
Here are my comments for this package. Please note that the bug that I pointed
in bug 453851#c19 needs fixed in order to build this package in koji.

- rpmlint "globus-callout-devel.x86_64: W: no-documentation" can be ignored.

? Where does the version number come from? I don't see it in the sources. And
why is the release number 0.x ?

! Please move the explanation of Source8 to where you define Source8.

* The file, containing the text of the license(s) for the package must be
included in %doc if (and only if) the source package includes the text of the
license(s) in its own file. The original source tree does not contain the
license file in the callout/source/ folder, so Source9 should be left out.

? Should the license be ASL 2.0? The source files say they are ASL 2.0 on their
headers.

* Description needs to be descriptive. What is a callout library?

* The new guidelines suggest that %global should be preferred over %define

! Could you collect all your "%global"s at one place?

! Please make the descriptions span 80 columns

? Why are you packaging the .filelist files?

* Fedora specific compiler flags are not honored in the linking phase. At the
least, "-g -Wall" needs passed.

* The doc package is fairly small. Why don't you put this documentation in the
devel subpackage?

! Please replace /usr/share with %{_datadir}

! Please explain the non-trivial things you do in the SPEC file with comments.
Why do you remove those files in %build? Why are those sed's for? etc

* On the main package:
    Requires:       globus-libtool >= 1
    BuildRequires:  globus-libtool-devel >= 1
    BuildRequires:  globus-core >= 4 ,
  on the devel subpackage:
    Requires:       globus-libtool-devel >= 1
    Requires:       globus-core >= 4
    Requires:       pkgconfig
  are redundant. They will be picked up by other dependencies.

! If what you want to do is to erase lines, you can replace
   for l in $RPM_BUILD_ROOT%{_datadir}/globus/packages/%{_name}/*.filelist ; do
      grep -v 'lib.*\.la$' < $l > $l.new
      mv $l.new $l
   done
with 
   sed -i '/lib.*\.la$/d' \
       $RPM_BUILD_ROOT%{_datadir}/globus/packages/%{_name}/*.filelist
to make the code simpler. Similarly for the .a files.

Also, the multiple instances of
   cat some_file |sed s!some!expression! > new_file
   mv new_file some_file
can be replaced by
   sed -i -e s!some!expression! \
          -e s!other!expression! \
          -e s!yetanother!expression! \
   some_file

These changes would shorten your SPEC file quite a bit. Again, explaining these
things in the SPEC file as comments would be a great bonus for reviewers and
other packagers who check your package.

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