Spec URL: http://svn.fedorahosted.org/svn/documentation-devel/trunk/Files/documentation-devel-Fedora.spec SRPM URL: http://svn.fedorahosted.org/svn/documentation-devel/trunk/Files/documentation-devel-Fedora-0.4-0.fc9.src.rpm Description: Fedora branded theme for docuemntation-devel.
Some initial comments: You can use %{_datadir} instead of %{_usr}/share. I don't think "BuildRequires: documentation-devel" is necessary. Personally I would prefer lowercase for the branch suffix: ie documentation-devel-fedora. That way it would be easier to find with globbing etc.
err, brand suffix, not branch.
BuildRequires: documentation-devel: po2entity from documentation-devel is used to create translated entity files at build time, so this BuildRequires is correct. Cheers, Jeff.
Would you consider using "-fedora" for the suffix instead of "-Fedora"?
(In reply to comment #4) > Would you consider using "-fedora" for the suffix instead of "-Fedora"? Sure, Fedora demoted from propern noun to plain old noun. I'll wait until the main package is accepted before supplying new spec and srpm, since I need to set the correct dependencies and rename this package to match.
http://svn.fedorahosted.org/svn/documentation-devel/trunk/Files/publican-fedora-0.6-0.fc9.src.rpm http://svn.fedorahosted.org/svn/documentation-devel/trunk/Files/publican-fedora.spec Updated for renamed master package.
Requires(post): coreutils Requires(postun): coreutils are not needed. Minor: "-n %{name}-%{version}" is superfluous for %setup.
http://svn.fedorahosted.org/svn/documentation-devel/trunk/Files/publican-fedora-0.8-0.fc9.src.rpm http://svn.fedorahosted.org/svn/documentation-devel/trunk/Files/publican-fedora.spec - Setup per Brand Book_Templates - Fix soure and URL paths - Use release in source path - add OPL text as COPYING - removed coreutils reqq
Updated urls are: Spec URL: http://svn.fedorahosted.org/svn/publican/trunk/Files/publican-fedora.spec SRPM URL: http://svn.fedorahosted.org/svn/publican/trunk/Files/publican-fedora-0.8-0.fc9.src.rpm
rpmlint says: publican-fedora.src:16: W: unversioned-explicit-obsoletes documentation-devel-Fedora This is ok, but probably it can be dropped assuming there aren't many people using the old package. publican-fedora.src: W: invalid-license Open Publication License The Fedora short license name is "Open Publication": see http://fedoraproject.org/wiki/Licensing#head-7c329132fff48be993272795da69b49c7812e8a9 A few comments: publican-0.29/Book_Template/en-US/ and publican-fedora-0.8/Book_Template/en-US are the same, but presumably they are duplicated because they are under different licenses, right? The README file says "for building Red_Hat_Enterprise_Linux documentation". It would be valuable to get comments on this package from the Fedora Docs project since I am not really qualified to evaluate the templates, etc. Otherwise the package basically looks ok to me.
(In reply to comment #10) > rpmlint says: > > publican-fedora.src:16: W: unversioned-explicit-obsoletes > documentation-devel-Fedora > > This is ok, but probably it can be dropped assuming there aren't many people > using the old package. > > publican-fedora.src: W: invalid-license Open Publication License > > The Fedora short license name is "Open Publication": see > http://fedoraproject.org/wiki/Licensing#head-7c329132fff48be993272795da69b49c7812e8a9 Fixed. > A few comments: > > publican-0.29/Book_Template/en-US/ and publican-fedora-0.8/Book_Template/en-US > are the same, but presumably they are duplicated because they are under > different licenses, right? This is correct. > The README file says "for building Red_Hat_Enterprise_Linux documentation". Changed to Fedora. > It would be valuable to get comments on this package from the Fedora Docs project > since I am not really qualified to evaluate the templates, etc. It's pretty hard for the Fedora docs team to comment on this since they can't actually use the system until it's in Fedora. I'm sure once it is in there and they can play with it a bit, they will have a bunch of changes to the look and feel, and the common content. In fact I'm hoping they will want to take over this package :) I don't think the content needs to be exact since no Fedora documentation uses this system yet. > Otherwise the package basically looks ok to me. http://svn.fedorahosted.org/svn/publican/trunk/Files/publican-fedora-0.8-2.fc9.src.rpm http://svn.fedorahosted.org/svn/publican/trunk/Files/publican-fedora.spec
I think it would be better to change the package Group from "Applications/Text" to something else like "Development/Libraries". I am not sure if there is a more appropriate group.
(In reply to comment #11) > It's pretty hard for the Fedora docs team to comment on this since they can't > actually use the system until it's in Fedora. > > I'm sure once it is in there and they can play with it a bit, they will have a > bunch of changes to the look and feel, and the common content. In fact I'm > hoping they will want to take over this package :) +1 That was the plan; no need to slow the packaging process down for looking down at that level. Once it's a package, then we can commence to arguing stuff with the upstream, like, "Can you switch everything to uses dashes '-' instead of underscores '_'?" I kid!! Actually, I've looked a bit at the source and have seen this system on-and-off over the years, and haven't seen any surprises. Using it for Fedora Docs is going to mean some potential modifications to the source XML (file names, other conventions), but that is expected. I reckon stuff will build with just a little fiddling.
(In reply to comment #11) > (In reply to comment #10) > > rpmlint says: > > publican-fedora.src: W: invalid-license Open Publication License > > > > The Fedora short license name is "Open Publication": see > > > http://fedoraproject.org/wiki/Licensing#head-7c329132fff48be993272795da69b49c7812e8a9 > > Fixed. The legal notice specifies " ... Open Publication License, V1.0 or later ..." My understanding from Red Hat Legal (discussion with Mark Webbink) is that we don't want to use the "or later" language because we don't want to commit to using something that doesn't even exist yet. I'm not sure if this matters. That can exist in the toolchain and not be the same in the doc itself, right? As upstream you have a range of licensing decisions you can make, but since it's actually RHT as the upstream, I recommend this more conservative language. If you like, you can swipe the lingo directly from the Fedora Docs legal notice: http://docs.fedoraproject.org/release-notes/f8/en_US/legalnotice.html http://docs.fedoraproject.org/release-notes/f8/en_US/sn-legalnotice.html The first is a pointer with set-up, so the set-up lingo matches what the main legal notice page has. The wording on the sn-legalnotice.html is directly from the OPL; the rest is a modified grandchild of the legal notice that used to be in RHEL docs, inherited through the original Fedora days.
Here is the review: +:ok, =:needs attention MUST Items: [=] MUST: rpmlint must be run on every package. publican-fedora.src:17: W: unversioned-explicit-obsoletes documentation-devel-Fedora publican-fedora.noarch: W: obsolete-not-provided documentation-devel-Fedora These are ok. publican-fedora.noarch: W: incoherent-version-in-changelog 0.8-1 0.8-2 Please take care of this one. [+] MUST: The package must be named according to the Package Naming Guidelines. [+] MUST: The spec file name must match the base package %{name}, in the format %{name}.spec unless your package has an exemption on Package Naming Guidelines. [=] MUST: The package must meet the Packaging Guidelines. Please remove the redundant Requires(post) and Requires(postun). [+] MUST: The package must be licensed with a Fedora approved license and meet the Licensing Guidelines. [+] MUST: The License field in the package spec file must match the actual license. [+] MUST: If (and only if) the source package includes the text of the license(s) in its own file, then that file, containing the text of the license(s) for the package must be included in %doc. (But please consider Karsten's comments above.) [+] MUST: The spec file must be written in American English. [+] MUST: The spec file for the package MUST be legible. [+] MUST: The sources used to build the package must match the upstream source f0d61bc6d241c57e03c91f40120c0e87 package-review/publican-fedora/publican-fedora-0.8.tgz [+] MUST: The package must successfully compile and build into binary rpms on at least one supported architecture. [+] MUST: All build dependencies must be listed in BuildRequires [+] MUST: A package must own all directories that it creates. [+] MUST: A package must not contain any duplicate files in the %files listing. [+] MUST: Permissions on files must be set properly. [+] MUST: Each package must have a %clean section, which contains rm -rf %{buildroot} (or $RPM_BUILD_ROOT). [+] MUST: Each package must consistently use macros, as described in the macros section of Packaging Guidelines. [+] MUST: The package must contain code, or permissable content. [+] MUST: If a package includes something as %doc, it must not affect the runtime of the application. [+] MUST: Packages must not own files or directories already owned by other packages. [+] MUST: At the beginning of %install, each package MUST run rm -rf %{buildroot} (or $RPM_BUILD_ROOT). See Prepping BuildRoot For %install for details. [+] MUST: All filenames in rpm packages must be valid UTF-8. SHOULD Items: [+] SHOULD: The reviewer should test that the package functions as described. A package should not segfault instead of running, for example. I tested "create_book --brand fedora --name ..." and a few obvious make targets, and it seems to work ok.
Created attachment 294760 [details] publican-fedora.spec-1.patch
I don't see any other problems with the package, but reviewers, feel free to post additional comments if you should spot anything I might have overlooked. Package is APPROVED, but please address the minor points raised above.
New Package CVS Request ======================= Package Name: publican-fedora Short Description: Publican documentation template files for fedora Owners: jfearn Branches: devel, F-8, EL-5, EL-4 InitialCC: mdious Cvsextras Commits: yes
cvs admin is done
built in koji http://koji.fedoraproject.org/koji/buildinfo?buildID=36125
Package Change Request ====================== Package Name: publican-fedora New Branches: el6-docs Owners: rlandmann
Invalid branch EL-6-docs requested