Bug 1394789

An informal review:
1. Group & BuildRoot tag should not be used, according to "Tags and Sections" at http://fedoraproject.org/wiki/Packaging:Guidelines
2. File lists is quite long. If all files under a folder are included, is it enough to include the folder only? that is, use:
%{_datadir}/nvim/runtime
instead of:
%dir %{_datadir}/nvim/runtime
3. Better to have rpmlint result and koji build link here.

I'm having 2 new packages under review, bug 1369708 and 1369720. Would like a review-swap if possible.

Spec URL: https://xor.cryptomilk.org/rpm/neovim/neovim.spec
SRPM URL: https://xor.cryptomilk.org/rpm/neovim/neovim-0.1.6-2.fc24.src.rpm

I've removed Group and BuildRoot. I prefer a complete file list, then I know what is going on. Which files get added and which have been removed.

Koji is not possible, see the dependencies.

We do not need lua-bitop it is a builtin of luajit which neovim uses.

Spec URL: https://xor.cryptomilk.org/rpm/neovim/neovim.spec
SRPM URL: https://xor.cryptomilk.org/rpm/neovim/neovim-0.1.7-1.fc25.src.rpm

Scratch build failed:
http://koji.fedoraproject.org/koji/taskinfo?taskID=16716812

By the way jemalloc-devel build requires is missing.

Spec URL: https://xor.cryptomilk.org/rpm/neovim/neovim.spec
SRPM URL: https://xor.cryptomilk.org/rpm/neovim/neovim-0.1.7-2.fc25.src.rpm

Scratch build: http://koji.fedoraproject.org/koji/taskinfo?taskID=16754838

Use gettext instead of gettext-devel.

Quick question. Do we need luajit-devel (we are using the Lua interpreter)?

One more thing:

BuildRequires: gcc

C and C++ packaging guidelines:
https://fedoraproject.org/wiki/Packaging:C_and_C%2B%2B#BuildRequires_and_Requires

This is simpler

%build
mkdir -p obj

than

%build
if test ! -e "obj"; then
  mkdir obj
fi

Well luajit would be better, but luajit assumes lua 5.1:

LuaJIT 2.0.4 -- Copyright (C) 2005-2015 Mike Pall. http://luajit.org/
JIT: ON CMOV SSE2 SSE3 SSE4.1 fold cse dce fwd dse narrow loop abc sink fuse
> require("lpeg")
stdin:1: module 'lpeg' not found:
        no field package.preload['lpeg']
        no file './lpeg.lua'
        no file '/usr/share/luajit-2.0.4/lpeg.lua'
        no file '/usr/local/share/lua/5.1/lpeg.lua'
        no file '/usr/local/share/lua/5.1/lpeg/init.lua'
        no file '/usr/share/lua/5.1/lpeg.lua'
        no file '/usr/share/lua/5.1/lpeg/init.lua'
        no file './lpeg.so'
        no file '/usr/local/lib/lua/5.1/lpeg.so'
        no file '/usr/lib64/lua/5.1/lpeg.so'
        no file '/usr/local/lib/lua/5.1/loadall.so'


So we need to use lua which is lua 5.3 and luajit seems to be incompatible.

Spec URL: https://xor.cryptomilk.org/rpm/neovim/neovim.spec
SRPM URL: https://xor.cryptomilk.org/rpm/neovim/neovim-0.1.7-3.fc25.src.rpm

Looks good.

I noticed something else. The license file is in %doc and should be in %license:

%files
%license LICENSE
%doc ...

I could swear I've already fixed that before ...

Spec URL: https://xor.cryptomilk.org/rpm/neovim/neovim.spec
SRPM URL: https://xor.cryptomilk.org/rpm/neovim/neovim-0.1.7-4.fc25.src.rpm

Fedora-review says:

Issues:
=======
- All build dependencies are listed in BuildRequires, except for any that
  are listed in the exceptions section of Packaging Guidelines.
  Note: These BR are not needed: gcc
  See: http://fedoraproject.org/wiki/Packaging/Guidelines#Exceptions_2

Remove the gcc dependency and I'll approve :)

"If your application is a C or C++ application you must list a BuildRequires against gcc, gcc-c++ or clang. Those packages will include everything that is required to build a standards conforming C or C++ application."

From: https://fedoraproject.org/wiki/Packaging:C_and_C%2B%2B#Packaging

OK, strange that fedora-review explicitly flags gcc. I don't have any more comments, then.

I was thinking of changing Recommends python{2,3}-neovim to Suggests.
This way we get fewer dependencies (Python packages or only suggested not installed). They are not necessary to run neovim.

What do you think guys?

Package request has been approved: https://admin.fedoraproject.org/pkgdb/package/rpms/neovim

neovim-0.1.7-4.fc25 has been submitted as an update to Fedora 25. https://bodhi.fedoraproject.org/updates/FEDORA-2016-b8f61d4abb

neovim-0.1.7-4.fc25 has been pushed to the Fedora 25 testing repository. If problems still persist, please make note of it in this bug report.
See https://fedoraproject.org/wiki/QA:Updates_Testing for
instructions on how to install test updates.
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2016-b8f61d4abb

neovim-0.1.7-4.fc25 has been pushed to the Fedora 25 stable repository. If problems still persist, please make note of it in this bug report.

neovim-0.1.7-4.fc25 has been pushed to the Fedora 25 stable repository. If problems still persist, please make note of it in this bug report.