Bug 226376 - Merge Review: rootfiles
Summary: Merge Review: rootfiles
Keywords:
Status: CLOSED RAWHIDE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Susi Lehtola
QA Contact: Fedora Package Reviews List
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2007-01-31 20:51 UTC by Nobody's working on this, feel free to take it
Modified: 2009-03-23 14:46 UTC (History)
4 users (show)

Fixed In Version:
Clone Of:
Environment:
Last Closed: 2009-03-23 14:20:08 UTC
Type: ---
Embargoed:
susi.lehtola: fedora-review+


Attachments (Terms of Use)

Description Nobody's working on this, feel free to take it 2007-01-31 20:51:42 UTC
Fedora Merge Review: rootfiles

http://cvs.fedora.redhat.com/viewcvs/devel/rootfiles/
Initial Owner: pknirsch

Comment 1 Susi Lehtola 2009-03-21 22:16:02 UTC
rpmlint output:
rootfiles.noarch: W: no-documentation
rootfiles.noarch: W: hidden-file-or-dir /root/.bashrc
rootfiles.noarch: W: hidden-file-or-dir /root/.cshrc
rootfiles.noarch: W: hidden-file-or-dir /root/.tcshrc
rootfiles.noarch: W: hidden-file-or-dir /root/.bash_logout
rootfiles.noarch: W: hidden-file-or-dir /root/.bash_profile
rootfiles.noarch: W: no-url-tag
rootfiles.noarch: W: non-etc-or-var-file-marked-as-conffile /root/.bash_logout
rootfiles.noarch: W: non-etc-or-var-file-marked-as-conffile /root/.bash_profile
rootfiles.noarch: W: non-etc-or-var-file-marked-as-conffile /root/.bashrc
rootfiles.noarch: W: non-etc-or-var-file-marked-as-conffile /root/.cshrc
rootfiles.noarch: W: non-etc-or-var-file-marked-as-conffile /root/.tcshrc
rootfiles.src: W: no-%build-section
rootfiles.src: W: no-url-tag
2 packages and 0 specfiles checked; 0 errors, 14 warnings.

All of these except the url tag warning are fine due to the special nature of the package.

- For the url tag, add the disclaimer available at
http://fedoraproject.org/wiki/Packaging/SourceURL#We_are_Upstream
to the spec file.

- Add %{?dist} to the release tag.

- Preserve time stamps with install -p.

After these are fixed the package is good to go.

Comment 2 Phil Knirsch 2009-03-23 12:40:55 UTC
- %{?dist} has already been added in a recent version
- Added the "we-are-upstream" comments above the sources
- Added the -p option to the install command to preserve timestamps

rootfiles-8.1-4 should contain all the requested fixes and has just been built in rawhide.

Thanks & regards, Phil

Comment 3 Susi Lehtola 2009-03-23 14:20:08 UTC
Review:
X source files match upstream (we are upstream)
X package meets naming and versioning guidelines.
X specfile is properly named, is cleanly written and uses macros consistently.
X dist tag is present.
X build root is correct.
X license field matches the actual license.
X license is open source-compatible.
X license text not included upstream.
X latest version is being packaged.
X BuildRequires are proper.
X compiler flags are appropriate.
X %clean is present.
X package builds in mock.
X package installs properly.
X debuginfo package looks complete.
X rpmlint is silent.
? final provides and requires are sane:
 * Is ncurses really necessary??
X no check available
X no shared libraries are added to the regular linker search paths.
X owns the directories it creates.
X doesn't own any directories it shouldn't.
X no duplicates in %files.
X file permissions are appropriate.
X no scriptlets present.
X code, not content.
X documentation is small, so no -docs subpackage is necessary.
X %docs are not necessary for the proper functioning of the package. 
X no headers.
X no pkgconfig files.
X no libtool .la droppings.
X no desktop files

I don't think the ncurses requirement should be there, since the bug only affects older distributions. Maybe add a conditional for older distros that are affected by the bug? On the other hand, this would make the spec unnecessarily complicated.

In any case, add a note about the ncurses requirement so that it may be dropped in the future.

****

The package has been found to adhere to the guidelines and is thus

APPROVED

Comment 4 Ondrej Vasik 2009-03-23 14:46:53 UTC
Maybe clear itself could be removed as well - and of course the ncurses requirements ( as discussed https://bugzilla.redhat.com/show_bug.cgi?id=223960 and https://bugzilla.redhat.com/show_bug.cgi?id=429406) and rootfiles generally updated to comply with bash provided all-users-files in /etc/skel...


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