vim-7.0.191-1.fc7 has lots of spec file fixes, please don't review older releases

/etc/profile.d/vim.{sh,csh} is 755, but should be 644:

W: vim-enhanced conffile-without-noreplace-flag /etc/profile.d/vim.csh
W: vim-enhanced conffile-without-noreplace-flag /etc/profile.d/vim.sh
E: vim-enhanced executable-marked-as-config-file /etc/profile.d/vim.sh
E: vim-enhanced executable-sourced-script /etc/profile.d/vim.sh 0755
E: vim-enhanced executable-marked-as-config-file /etc/profile.d/vim.csh
E: vim-enhanced executable-sourced-script /etc/profile.d/vim.csh 0755

fixed in vim-7.0.195-2.fc7

What's the status of this review ? Approved ?

Hi Karsten,

Needs work:
* BuildRequires: perl should not be included
  (wiki: PackagingGuidelines#Exceptions)
* Spec file: some paths are not replaced with RPM macros
  (wiki: QAChecklist item 7)
* rpmlint of vim-common: 
* The package should contain the text of the license
  (wiki: Packaging/ReviewGuidelines)
* Each %files section should have a %defattr line
  (wiki: Packaging/ReviewGuidelines)
* Desktop file: vendor should be fedora
  (wiki: PackagingGuidelines#desktop)
* Scriptlets: missing update-desktop-database
  (wiki: ScriptletSnippets)
* The package owns /usr/share/man/fr, which is a standard directory
  (wiki: Packaging/ReviewGuidelines)
* The package owns /usr/share/man/fr/man1, which is a standard directory
  (wiki: Packaging/ReviewGuidelines)
* The package owns /usr/share/man/it, which is a standard directory
  (wiki: Packaging/ReviewGuidelines)
* The package owns /usr/share/man/it/man1, which is a standard directory
  (wiki: Packaging/ReviewGuidelines)
* The package owns /usr/share/man/pl, which is a standard directory
  (wiki: Packaging/ReviewGuidelines)
* The package owns /usr/share/man/pl/man1, which is a standard directory
  (wiki: Packaging/ReviewGuidelines)

Some minor things:
* Please use BuildRequires instead of Buildrequires everywhere
* Preserve timestamps when installing non-generated files
* Please run rpmlint on all rpms and fix the warnings and errors. The list is a bit long to include here.

ok, I'm now down to just a few warnings.
Most of them are like this one:
vim-common file-not-utf8 /usr/share/man/ru.KOI8-R/man1/vimdiff.1.gz

This file isn't intented to be UTF8, it's KOI8-R encoded.

vim-enhanced and vim-X11 report dangling symlinks, but this becomes a normal 
symlink when the required vim-common is installed.

vim -minimal has no documentation, I'd like to keep it this way. Maybe I'll add
the license, but nothing more.

Hi Karsten,

> ok, I'm now down to just a few warnings.
> Most of them are like this one:
> vim-common file-not-utf8 /usr/share/man/ru.KOI8-R/man1/vimdiff.1.gz

> This file isn't intented to be UTF8, it's KOI8-R encoded.

That makes sense :-)

> vim-enhanced and vim-X11 report dangling symlinks, but this becomes a normal 
> symlink when the required vim-common is installed.

OK

> vim -minimal has no documentation, I'd like to keep it this way. Maybe I'll add
> the license, but nothing more.

The License file would be nice.

Just two small things:
- vim-X11 BuildRequires libSM-devel, but that one is already required by libXt-devel
- Requires: /usr/bin/desktop-file-install on line 305, replace /usr/bin with %{_bindir}

Karsten, why are you shipping man pages in UTF-8 AND in ISO8859-X/KOI8-R. IMHO 
this doesn't make sense, UTF-8 should be enough and the stuff should put be in 
the man pages directory without any .CHARSET extension.

(In reply to comment #8)
> Karsten, why are you shipping man pages in UTF-8 AND in ISO8859-X/KOI8-R. IMHO 
> this doesn't make sense, UTF-8 should be enough and the stuff should put be in 
> the man pages directory without any .CHARSET extension.

It does make sense. For those who use a non UTF8 encoded 
locale. I don't know exactly how man select the man page 
based on the locale encoding, maybe it doesn't really work 
right, but if it is broken man should be fixed, not the man 
pages.

Well...UTF-8 is the Fedora default charset, anything else is normally not 
likely seen. But finally, please put the UTF-8 to the man directory itself
and not into .UTF-8 directory as UTF-8 is really the Fedora default charset.

Nevertheless the abused directories have to be provided either by the man,
man-pages or the filesystem package! :)

Ping?

Review for release 1.fc10:
* RPM name is OK
* Source vim-7.2.tar.bz2 is the same as upstream
* Source vim-7.2-lang.tar.gz is the same as upstream
* Source vim-7.2-extra.tar.gz is the same as upstream
* Source forth.vim is the same as upstream
* This is the latest version
* Builds fine in mock
* rpmlint of vim-minimal looks OK
* rpmlint of vim-X11 looks OK
* rpmlint of vim-debuginfo looks OK
* rpmlint of vim-enhanced looks OK
* rpmlint of vim-common looks OK
* File list of vim-minimal looks OK
* File list of vim-X11 looks OK
* File list of vim-debuginfo looks OK
* File list of vim-enhanced looks OK
* File list of vim-common looks OK
* Config files of vim-minimal looks OK
* Config files of vim-enhanced looks OK
* Config files of vim-common looks OK

Needs work:
* Desktop file: the Categories tag should not contain Application any more
  (wiki: Packaging/Guidelines#desktop)
* Desktop file: the category Application is not valid
  (http://standards.freedesktop.org/menu-spec/latest/apa.html)
* As vim ships icons in the hicolor directory, it should have "Requires: hicolor-icon-theme"  https://www.redhat.com/archives/fedora-extras-list/2006-September/msg00282.html
* A few rpmlint warnings:
[ruben@slice vim]$ rpmlint vim-common-7.2.022-1.fc10.x86_64.rpm | grep spurious
vim-common.x86_64: W: spurious-executable-perm /usr/share/doc/vim-common-7.2.022/README_ami.txt.info
vim-common.x86_64: W: spurious-executable-perm /usr/share/doc/vim-common-7.2.022/README.txt.info
vim-common.x86_64: W: spurious-executable-perm /usr/share/doc/vim-common-7.2.022/README_amibin.txt.info
vim-common.x86_64: W: spurious-executable-perm /usr/share/doc/vim-common-7.2.022/README_amisrc.txt.info

* Like Robert said in comment #11, the man directories are not owned by any package, please resolve this.

I've built a new vim version in koji with fixes for those issues.
Please have a look at the packages in http://kojipkgs.fedoraproject.org/packages/vim/7.2.025/1.fc10/

Thanks Karsten,

You've commented out the line below, probably accidental?
%clean
#rm -rf $RPM_BUILD_ROOT

Furthermore all issues have been resolved, package is APPROVED.
Thanks for your help.

yes, I forgot to re-enable that. Thanks for catching that one, I'll build a fixed package.