Bug 196281 (php-manual-en) - Review Request: php-manual-en - English language PHP manual
Summary: Review Request: php-manual-en - English language PHP manual
Keywords:
Status: CLOSED NEXTRELEASE
Alias: php-manual-en
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Christopher Stone
QA Contact: Fedora Package Reviews List
URL:
Whiteboard:
Depends On:
Blocks: FE-ACCEPT
TreeView+ depends on / blocked
 
Reported: 2006-06-22 13:20 UTC by Tim Jackson
Modified: 2007-11-30 22:11 UTC (History)
2 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2006-08-19 11:32:31 UTC
Type: ---
Embargoed:


Attachments (Terms of Use)

Description Tim Jackson 2006-06-22 13:20:53 UTC
Spec URL: http://www.timj.co.uk/linux/specs/php-manual-en.spec
SRPM URL: http://www.timj.co.uk/linux/srpms/php-manual-en-20060527-1.src.rpm
Description: A documentation-only package containing the English language version of the PHP manual, split into a file per chapter.

See thread starting:
https://www.redhat.com/archives/fedora-extras-list/2006-May/msg00237.html

for a discussion of a number of issues relating to this package.

A few quick points:

- rpmlint says:

W: php-manual-en invalid-license Open Publication License v1.0

I think this is OK, since the above string is the correct and most accurate description of the package license

- Arguably the package should be php-manual-en-us, since in theory there might be an en_GB manual at some point, but I wonder whether this is going too far with the whole abstraction

- the Source URL is not really right for two reasons:

a) it has to have a silly /mirror/rubbish/junk on the end
b) it doesn't include the version in it

but this is an upstream problem.

Comment 1 Hugo Cisneiros 2006-06-22 13:34:46 UTC
(In reply to comment #0)
> W: php-manual-en invalid-license Open Publication License v1.0

Just "OPL" would be better?

> - Arguably the package should be php-manual-en-us, since in theory there
> might be an en_GB manual at some point, but I wonder whether this is going
> too far with the whole abstraction

Some other languages really requires the xx_YY naming... The package could 
contain php manuals from other languages too, don't you think? A source 
package could crate some packages, one for each language.

About the en vs. en-us, I think you should follow upstream on this. So 
only 'en' is IMO the right thing.

> - the Source URL is not really right for two reasons:
> 
> a) it has to have a silly /mirror/rubbish/junk on the end
> b) it doesn't include the version in it
> 
> but this is an upstream problem.

You should download and get the exact URL, for example:
http://us2.php.net/distributions/manual/php_manual_en.tar.gz



Comment 2 Tim Jackson 2006-06-26 14:34:54 UTC
> > W: php-manual-en invalid-license Open Publication License v1.0
> Just "OPL" would be better?

It doesn't include a version. We really ought to specify a version if the
package specifies one, because otherwise if there's (say) an OPL v3 in future
with different terms, we would be misleading users by implying they could
distribute it under the terms of OPL v3 whereas the authors might have only
specified OPL v1. We should respect the author's license. Note for example that
recent Core "php" packages have started explicitly mentioning license version.

That said, "OPL v1.0" is fine by me, but I'm pretty sure rpmlint doesn't know
that either and it's less descriptive.
 
> > - Arguably the package should be php-manual-en-us, since in theory there
> > might be an en_GB manual at some point, but I wonder whether this is going
> > too far with the whole abstraction
> Some other languages really requires the xx_YY naming... 

That's ok, they can name it "php-manual-pt-br" or whatever as necessary.

I could maybe make the current package Provide php-manual-en-us, for forward
compatibility in case it gets split in future. That would make sense.

> The package could contain php manuals from other languages too, don't you 
> think? 

Yes, but see linked thread: I don't want the responsibility of maintaining them.
Also if I included every language supplied by PHP then the source RPM would be
really huge.
Additionally in future it's possible that not all languages would get updated at
the same time, so it could in theory mean pushing a big update SRPM even though
many sources are unchanged.

> You should download and get the exact URL, for example:
> http://us2.php.net/distributions/manual/php_manual_en.tar.gz

Thanks, I didn't pick up on that direct URL.

Updated files:

Spec URL: http://www.timj.co.uk/linux/specs/php-manual-en.spec
SRPM URL: http://www.timj.co.uk/linux/srpms/php-manual-en-20060527-2.src.rpm


Comment 3 Christopher Stone 2006-07-08 05:21:16 UTC
Use %{_defaultdocdir} instead of %{_datadir}/doc

Use %{_defaultdocdir}/php-manual instead of %{_defaultdocdir}/php-manual/en in
your %files section.




Comment 4 Tim Jackson 2006-07-08 16:18:10 UTC
(In reply to comment #3)

> Use %{_defaultdocdir}/php-manual instead of %{_defaultdocdir}/php-manual/en in
> your %files section.

How does this take account of the i18n issues raised in the linked thread?



Comment 5 Christopher Stone 2006-07-08 19:16:58 UTC
There is no clear hierachry of directory ownership of the
%{_defaultdocdir}/php-manual so all packages that use this directory should own
this directory.

Unless there is some other issue I am missing?  I'm not sure what you are
referring to when you say "linked thread".

Comment 6 Tim Jackson 2006-07-09 17:24:15 UTC
(In reply to comment #5)

Are you trying to say I should *add* a %dir %{_defaultdocdir}/php-manual? If so,
yep, good call.

What you implied was that the docs should go in %{_defaultdocdir}/php-manual
instead of %{_defaultdocdir}/php-manual/en , which seems to not cater for
further internationalisation (e.g. future packages providing ....php-manual/fr
for example.

"Linked thread" refers to the thread mentioned in the bug description, namely:

https://www.redhat.com/archives/fedora-extras-list/2006-May/msg00237.html

Comment 7 Christopher Stone 2006-07-09 22:47:47 UTC
Yes this is what I'm trying to say. You can add the %dir, or I think just saying
%{_defaultdocdir}/php-manual in the %files section will accomplish the same
thing, either way you want to code it is fine.

Comment 8 Tim Jackson 2006-08-05 23:19:48 UTC
I think all this is sorted.

- rpmlint is quiet
- updated to latest ver
- docdir ownership is good

Spec URL: http://www.timj.co.uk/linux/specs/php-manual-en.spec
SRPM URL: http://www.timj.co.uk/linux/srpms/php-manual-en-20060725-1.src.rpm


Comment 9 Christopher Stone 2006-08-10 21:33:19 UTC
-rpmlint output clean
-package named according to Package Naming Guidelines
-spec file name matches package %{name}
-package licensed with open-source compatible license

O I could not find the license file in the package or on the web site.  Please
let me know where I can find the upstream license

-license file not included in %doc
-spec file written in American English
-spec file legible
-source matches upstream
a8ef43e9ff1da2ce18df90e67fc5758f  php_manual_en.tar.gz
-package successfully compiles and builds on FC-5 x86_64
-package does not need any BuildRequires
-spec file does not contain any locales
-package does not contain any shared libraries
-package is not relocatable
-package owns all directories it creates
-package does not contain any duplicate files in %files
-file permissions set properly
-package contains proper %clean section
-macro usage is consistent
-package contains permissible content
-use of -doc subpackage does not make sense in this context since package is
already a doc package
-package does not include %doc
-package does not include header files or static libraries
-package does not use pkgconfig
-package does not contain any .so files
-package does not build a -devel subpackage
-package does not contain any .la files
-package is not a GUI needing a .desktop file
-package does not own files or directories owned by other packages


==== MUST ====
- Explain where documentation is licensed

==== SHOULD ====
- include a license file and place it in %doc

Comment 10 Tim Jackson 2006-08-11 07:41:36 UTC
copyright.html describes the distribution license
opl.license.html and linked pages are the license itself

I'd be tempted to symlink these into %{doc}, except that they are generated HTML
pages that would have broken links out of context.

How does creating a "placeholder" LICENSE file in %{doc} that says "For
licensing information please see %{defaultdocdir}/php-manual/en/copyright.html"
sound?

Comment 11 Christopher Stone 2006-08-14 19:20:05 UTC
Actually, this is good enough.  I just needed to know where it was located in
order to complete the review.  You may want to add a comment in the spec file
indicating where the license is located, but not necessary.

Approved as is.

Comment 12 Tim Jackson 2006-08-19 11:32:31 UTC
Imported & built - job #14355. Thanks for all the input!


Note You need to log in before you can comment on or make changes to this bug.