Bug 1245604

Update: removed unneeded rm command in %postun section

Spec URL: https://nforro.fedorapeople.org/vim-go.spec
SRPM URL: https://nforro.fedorapeople.org/vim-go-1.0.5-2.fc22.src.rpm

Update: require golang
        added command to %postun to clear vim tags

Spec URL: https://nforro.fedorapeople.org/vim-go.spec
SRPM URL: https://nforro.fedorapeople.org/vim-go-1.0.5-3.fc22.src.rpm

Looks good.

- Name matches https://fedoraproject.org/wiki/Packaging:NamingGuidelines#Addon_Packages_.28General.29
- license is OK

License file is not installed. Add %license LICENSE
[https://fedoraproject.org/wiki/Packaging:LicensingGuidelines#License_Text].

%post/%postun scriplets should be suffixed with || : so that they cannot fail.

I'm not a vim expert, but comparing the scriptlets with vim-taglist I see that vim-taglist removes the file, but you truncate it... Is this intentional?

I don't think use should mark %{vimfiles_root}/doc/* with %doc. Packages are not allowed to use stuff marked with %doc at runtime, but this help is used by vim itself, no?

Who owns %{vimfiles_root}/autoload, %{vimfiles_root}/compiler, %{vimfiles_root}/ftdetect, etc? Most likely your package should (co-)own those directories.

(In reply to Zbigniew Jędrzejewski-Szmek from comment #3)
> License file is not installed. Add %license LICENSE
> [https://fedoraproject.org/wiki/Packaging:LicensingGuidelines#License_Text].
>

There is no license file in the tarball. It was added to upstream master branch later, so it will probably be included in the next release.
 
> %post/%postun scriplets should be suffixed with || : so that they cannot
> fail.
> 

Fixed.

> I'm not a vim expert, but comparing the scriptlets with vim-taglist I see
> that vim-taglist removes the file, but you truncate it... Is this
> intentional?
> 

With rm, rpmlint complains about dangerous command in %postun section.

> I don't think use should mark %{vimfiles_root}/doc/* with %doc. Packages are
> not allowed to use stuff marked with %doc at runtime, but this help is used
> by vim itself, no?
> 

That's right, fixed.

> Who owns %{vimfiles_root}/autoload, %{vimfiles_root}/compiler,
> %{vimfiles_root}/ftdetect, etc? Most likely your package should (co-)own
> those directories.

Those directories are owned by vim-filesystem package, which is "artificial filesystem" package and it also exists in vim-go's dependency chain, so vim-go shouldn't co-own them.

Updated files:
Spec URL: https://nforro.fedorapeople.org/vim-go.spec
SRPM URL: https://nforro.fedorapeople.org/vim-go-1.0.5-4.fc22.src.rpm

I would suggest to add .metainfo.xml for Gnome Software [1]. You can find inspiration in my vim-command-t package.



[1] https://fedoraproject.org/wiki/Packaging:AppData#.metainfo.xml_file_creation
[2] http://pkgs.fedoraproject.org/cgit/vim-command-t.git/tree/

(In reply to Nikola Forró from comment #4)
> (In reply to Zbigniew Jędrzejewski-Szmek from comment #3)
> > License file is not installed. Add %license LICENSE
> > [https://fedoraproject.org/wiki/Packaging:LicensingGuidelines#License_Text].
> >
> 
> There is no license file in the tarball. It was added to upstream master
> branch later, so it will probably be included in the next release.
OK.

> > I'm not a vim expert, but comparing the scriptlets with vim-taglist I see
> > that vim-taglist removes the file, but you truncate it... Is this
> > intentional?
> With rm, rpmlint complains about dangerous command in %postun section.
rpmlint is quite often wrong. But if your version works, that is fine.

> > Who owns %{vimfiles_root}/autoload, %{vimfiles_root}/compiler,
> > %{vimfiles_root}/ftdetect, etc? Most likely your package should (co-)own
> > those directories.
> 
> Those directories are owned by vim-filesystem package, which is "artificial
> filesystem" package and it also exists in vim-go's dependency chain, so
> vim-go shouldn't co-own them.
Pff, I checked that with repoquery but must have messed something up.
It is as you say.

(In reply to Vít Ondruch from comment #5)
> I would suggest to add .metainfo.xml for Gnome Software [1]. You can find
> inspiration in my vim-command-t package.
Good point, please add the metadata file.

I now filed bugs against other vim plugins to do the same: https://bugzilla.redhat.com/buglist.cgi?quicksearch=vim%20%22please%20add%20appdata%20metainfo%20file%22 .

> Updated files:
> Spec URL: https://nforro.fedorapeople.org/vim-go.spec
> SRPM URL: https://nforro.fedorapeople.org/vim-go-1.0.5-4.fc22.src.rpm
- license is OK
- name is OK
- description is OK
- license file is not present but will be added in next release
- layout is good and all directories have ownership
- spec file is good
- scriptlets are present
- rpmlint only false positive typos
- requires and provides look OK

Package is APPROVED. Please add the metadata file.

One more thing: golang is not available everywhere, so your package should not be built for architectures where the dependencies cannot be satisfied anyway:

ExclusiveArch: %{go_arches} noarch

(taken from https://fedoraproject.org/wiki/PackagingDrafts/Go#Libraries_and_Arch)

(In reply to Vít Ondruch from comment #5)
> I would suggest to add .metainfo.xml for Gnome Software [1]. You can find
> inspiration in my vim-command-t package.
> 
> 
> 
> [1]
> https://fedoraproject.org/wiki/Packaging:AppData#.metainfo.xml_file_creation
> [2] http://pkgs.fedoraproject.org/cgit/vim-command-t.git/tree/
Added.

(In reply to Zbigniew Jędrzejewski-Szmek from comment #7)
> One more thing: golang is not available everywhere, so your package should
> not be built for architectures where the dependencies cannot be satisfied
> anyway:
> 
> ExclusiveArch: %{go_arches} noarch
> 
> (taken from
> https://fedoraproject.org/wiki/PackagingDrafts/Go#Libraries_and_Arch)
Fixed.

Spec URL: https://nforro.fedorapeople.org/vim-go.spec
Metainfo URL: https://nforro.fedorapeople.org/vim-go.metainfo.xml
SRPM URL: https://nforro.fedorapeople.org/vim-go-1.0.5-5.fc22.src.rpm

New Package SCM Request
=======================
Package Name: vim-go
Short Description: Go development plugin for Vim
Upstream URL: https://github.com/fatih/vim-go
Owners: nforro jchaloup
Branches: f21 f22 f23

Git done (by process-git-requests).

vim-go-1.1-1.fc21 has been submitted as an update for Fedora 21.
https://admin.fedoraproject.org/updates/vim-go-1.1-1.fc21

vim-go-1.1-1.fc22 has been submitted as an update for Fedora 22.
https://admin.fedoraproject.org/updates/vim-go-1.1-1.fc22

Thanks for the review.

vim-go-1.1-1.fc21 has been pushed to the Fedora 21 stable repository.

vim-go-1.1-1.fc22 has been pushed to the Fedora 22 stable repository.