Bug 217836 - Review Request: vim-vimoutliner - set of vim macros for editing outlines
Review Request: vim-vimoutliner - set of vim macros for editing outlines
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Tomas Mraz
Fedora Package Reviews List
:
Depends On:
Blocks: FE-ACCEPT
  Show dependency treegraph
 
Reported: 2006-11-30 05:13 EST by Matěj Cepl
Modified: 2007-11-30 17:11 EST (History)
2 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2007-01-09 10:21:31 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)

  None (edit)
Description Matěj Cepl 2006-11-30 05:13:06 EST
Spec URL: http://www.ceplovi.cz/matej/progs/rpms/vimoutliner.spec
SRPM URL: http://www.ceplovi.cz/matej/progs/rpms/vimoutliner-0.3.4-4.src.rpm
Description:
Vimoutliner provides commands for building using the Vim text editor as an
outline editor. For more explanation on what outlines are and what they are
good for see the script's webpage at
http://www.vimoutliner.org and the general discussion of outlines on
http://www.troubleshooters.com/tpromag/199911/199911.htm.
Comment 1 Matěj Cepl 2006-11-30 05:15:44 EST
And yes this is one of the first packages I made for Fedora and the first one
which I try to push into Extras.
Comment 2 Parag AN(पराग) 2006-12-02 14:12:37 EST
Got rpmlint errors
W: vimoutliner macro-in-%changelog doc
Macros are expanded in %changelog too, which can in unfortunate cases lead
to the package not building at all, or other subtle unexpected conditions that
affect the build.  Even when that doesn't happen, the expansion results in
possibly "rewriting history" on subsequent package revisions and generally
odd entries eg. in source rpms, which is rarely wanted.  Avoid use of macros
in %changelog altogether, or use two '%'s to escape them, like '%%foo'.

W: vimoutliner mixed-use-of-spaces-and-tabs (spaces: line 3, tab: line 14)
The specfile mixes use of spaces and tabs for indentation, which is a
cosmetic annoyance.  Use either spaces or tabs for indentation, not both.
Comment 3 Matěj Cepl 2006-12-03 11:25:44 EST
Thank you very much. Oh well, only now I found that I have run rpmlint against
_binary_ packages. Oh well, oh well.

Fixed package is available on
http://www.ceplovi.cz/matej/progs/rpms/vimoutliner-0.3.4-5.src.rpm
and spec file is still in the same location
http://www.ceplovi.cz/matej/progs/rpms/vimoutliner.spec

Any more comments?
Comment 4 Parag AN(पराग) 2006-12-03 11:41:09 EST
Unable to view SPEC and SPRM
You don't have permission to access /matej/progs/rpms/vimoutliner.spec on this
server.
Comment 5 Matěj Cepl 2006-12-04 08:43:55 EST
SPEC (http://www.ceplovi.cz/matej/progs/rpms/vimoutliner.spec) should be all
right, tested just now. SRPM was updated to new release and it is here
http://www.ceplovi.cz/matej/progs/rpms/vimoutliner-0.3.4-6.src.rpm
(and yes, I can download it through webbrowser as well).
Comment 6 Mamoru TASAKA 2006-12-08 09:37:40 EST
A quick glance at your spec file and....

* Please specify all sources.

* cp -f %{SOURCE5} README.Fedora
  - Use 'cp -p -f' to keep timestamp.

* Why does this package call "update-desktop-database" though
  no desktop file is included?

* helpztags %{_datadir}/vim/vimfiles/doc
  - What does this do?
  = If this script creates some files, the files (created by helpztags)
    should be included in this package with marked as 
    %ghost %verify(not md5 size mtime), for example.
  = If this script changes some files included in this package, the
    files to be modified should marked with %verify(...)
  = If this script changes some files included in other packages,
    it shouldn't unless the files are marked with %verify(not ...) or
    %config or so.

* %preun
  helpztags %{_datadir}/vim/vimfiles/doc
  - What does this do? At %preun stage, all files in this package
    still exist, so I think this does nothing (though I don't
    know what helpztags actually does......)

By the way, does any sponsor watching this?
Comment 7 Mamoru TASAKA 2006-12-08 09:41:22 EST
(In reply to comment #6)
> * Please specify all sources.
I meant "Please specify URLs of all sources"
Comment 8 Tomas Mraz 2006-12-11 06:22:15 EST
I'll sponsor Matej as soon as this passes a review.

Mamoru, thanks for co-reviewing this package.
Comment 9 Matěj Cepl 2006-12-11 09:53:00 EST
1) I made comments to the .spec file why some Sources don't have URL (e.g., do
you want
http://www.vimoutliner.org/modules.php?op=modload&name=Downloads&file=index&req=getit&lid=16
for otl2html?)
2) I use 'cp -p -f' to keep timestamp for README.Fedora, but I am not sure how
much worth it is -- the file is written and maintained as part of the package.

3) Yes, calling update-desktop-database was just a bug. Removed.

4) Business with helpztags -- I have talked about that with karsten (maintainer
of vim) and we came to the conclusion that I should use just vim command which
is now in .spec.

Updated files are
http://www.ceplovi.cz/matej/progs/rpms/vimoutliner-0.3.4-7.src.rpm and location
of .spec hasn't changed.
Comment 10 Matěj Cepl 2006-12-11 11:21:41 EST
There is a problem with ownership of /usr/share/vim/vimfiles/*/ directories,
which has to be fixed in co-operation with vim maintainer (see bug 219154).
Comment 11 Matěj Cepl 2006-12-14 07:38:00 EST
I got WONTFIX on that vim bug, so I think I cannot do with this bug nothing else
than just go ahead, and let this problem manifest itself later.
Comment 12 Kevin Fenzi 2006-12-16 14:27:46 EST
This review request should probibly be in the 'REVIEW' state, not the 'NEW' state. 
Fixing the blocking bugs for that...

I see that the bug referred to in comment #10 is closed WONTFIX. 
Is there a solution to the problem there?
Comment 13 Matěj Cepl 2006-12-18 02:31:34 EST
I should probably not use private comments in Extras bugs -- see comment 11.
Sorry for that.
Comment 14 Tomas Mraz 2007-01-05 12:17:14 EST
- naming guidelines request naming the package vim-vimoutliner which seems a
little bit weird but on Debian it is as well
Comment 16 Matěj Cepl 2007-01-08 10:21:59 EST
Does not work without

filetype plugin on

to /etc/vimrc (or ~/.vimrc). The way how to add this to vim needs to be
discussed with vim maintainer.
Comment 17 Tomas Mraz 2007-01-08 11:28:22 EST
OK, so vim maintainer agrees to add it in future vim updates. For now it is
added in %post script.

Final rewiew:

Package conforms to all MUST items, spec file is fine, rpmlint is silent.

Package builds fine here and seems to work as intended.

APPROVED
Comment 18 Mamoru TASAKA 2007-01-12 13:17:01 EST
(Changing summary for traceability)

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