[Bug 975309] Review Request: libcutl - C++ utility library from Code Synthesis

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

 



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

--- Comment #6 from Dave Johansen <davejohansen@xxxxxxxxx> ---
The updated files can be found at:
Spec URL: http://daveisfera.fedorapeople.org/odb_2.2/specs/libcutl.spec
SRPM URL:
http://daveisfera.fedorapeople.org/odb_2.2/SRPMS/libcutl-1.7.1-1.fc19.src.rpm

OK, this is a lot of info, but here's all of the comments and the response.
Hopefully, it's organized well enough to be readable/manageable.

>  * Your links are not direct linking.  You should upload them to somewhere
>    direct linking is possible.  If you don't have own webspace for this, you
>    possibly want to follow the instructions given here:
>    http://fedoraproject.org/wiki/Join_the_package_collection_maintainers#Upload_Your_Package
>    After your request got approved, upload spec/srpm to there and update the
>    Spec/SRPM URL-Tag in your bugs, please.

I tried following those instructions, but they appear to be outdated and the
link goes to Trac page that doesn't appear related to setting up hosting. Now,
that I'm sponsored, I put them in my fedorapeople.org location, but what's the
proper instructions? I wouldn't mind updating the instructions if I knew what
the right answer was.


>  * There are lots of obsolete BRs is the spec.  Remove them, please, so the
>    spec-file focusses on the real, additionally needed ones.  See:
>    https://fedoraproject.org/wiki/Packaging/Guidelines#Exceptions_2

Fixed, but it still lists binutils, glibc-common, net-tools and pkgconfig
because auto-buildrequires recommended them. Should I include those? Or just
remove them?


>  * doc-subpkg explicitly requires main-pkg.  This should not be the case,
>    because the docs don't need binaries.  Usually someone wants to read
>    the docs BEFORE installing the software.

Removed doc subpkg and included it with the main package based on other
feedback.


>  * For the above reason LICENSE should be packaged withing doc-subpkg, too.
>    According to the guidelines this is OK.  See:
>    https://fedoraproject.org/wiki/Packaging/Guidelines#Duplicate_Files

N/A based on removal of doc subpkg.


>  * %defattr(-,root,root,-) is obsoleted and was only needed on rpm < 4.4.
>    Even el5 shippes a more recent version of rpm.  See:
>    https://fedoraproject.org/wiki/Packaging/Guidelines#File_Permissions

Fixed. Sorry I read this in the guidelines before submitting, but it just
didn't click that I needed to remove that line. Could this check be added to
rpmlint?


> * Run "rpmlint -I" on all packages, the src.rpm *and* the built rpms. Apply fixes for obvious errors/warnings, ignore false positives, preferably comment on what rpmlint reports.

Did this and the only warnings now are about the spelling of "runtime" which
appears to be a valid spelling ( http://en.wikipedia.org/wiki/Runtime_library
). Is it ok to just ignore these warnings and leave the text as is because
that's how it appear upstream? If so, then is it possible to get "runtime"
added as a valid spelling in the check that rpmlint does?


> Rather "System Environment/Libraries" for base library packages, since those contain run-time libs as opposed to -devel packages.

Fixed.


> This needs a closer look, because the file LICENSE contains the MIT terms, and several source files mention MIT licensing, too.

Now built with --with-external-boost so that it doesn't use the Boost code that
is used when external Boost is not used and the license is just MIT as the
.spec file now states. The source rpm also includes a patch to remove the
mention of the boost code/license from the LICENSE file that is included as
documentation.


> Hmmm, I'm certain lots of reviewers would insist on having you remove these. Saying "it's easy" isn't convincing, because it's even easier to not list anything expected to be available within the minimum build environment:
> http://fedoraproject.org/wiki/Packaging:Guidelines#Exceptions_2

Fixed based on previous feedback.


> Separate documentation -doc packages typically don't require the base package. It should be possible to install documentation without having to install a program and all its dependencies.

Removed -doc packages based on other feedback.


> The *.so symlink belongs into the -devel package, because typically it's only needed/used when building software:
> http://fedoraproject.org/wiki/Packaging:Guidelines#Devel_Packages

> There are only few exceptions where external programs would dlopen a library via the non-versioned symlink (expect a few symbols only) and could not be patched to open the versioned lib instead.

Fixed


> Directory /usr/include/cutl is not included.

> http://fedoraproject.org/wiki/Packaging:Guidelines#File_and_Directory_Ownership
> http://fedoraproject.org/wiki/Packaging:UnownedDirectories

Based on those two sites, my understanding was that the issue was that the
directories could become orphaned, so I believe that I have fixed this now by
including them in the files section. Please let me know if there's something
else/more that I need to do for this.


> Further, the size of the libcutl-doc package is 5496 bytes. Unless large API docs are missing, I wouldn't split off these few and tiny files into a separate -doc package, but include them within the base package.
> http://fedoraproject.org/wiki/Packaging:Guidelines#Documentation

Included the documentation in the base package and removed the doc subpkg. I
removed all of the files that I thought were unnecessary (INSTALL, README,
version) and then added NEWS as the %doc of the -devel packages. This seemed
like the idea that most closely matched my understanding of the packaging
guidelines, but please let me know if I should change it further.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
Unsubscribe from this bug https://bugzilla.redhat.com/token.cgi?t=4rwkvioJoi&a=cc_unsubscribe
_______________________________________________
package-review mailing list
package-review@xxxxxxxxxxxxxxxxxxxxxxx
https://admin.fedoraproject.org/mailman/listinfo/package-review





[Index of Archives]     [Fedora Legacy]     [Fedora Desktop]     [Fedora SELinux]     [Yosemite News]     [KDE Users]     [Fedora Tools]