Bug 217836
| Summary: | Review Request: vim-vimoutliner - set of vim macros for editing outlines | ||
|---|---|---|---|
| Product: | [Fedora] Fedora | Reporter: | Matěj Cepl <mcepl> |
| Component: | Package Review | Assignee: | Tomas Mraz <tmraz> |
| Status: | CLOSED NEXTRELEASE | QA Contact: | Fedora Package Reviews List <fedora-package-review> |
| Severity: | medium | Docs Contact: | |
| Priority: | medium | ||
| Version: | rawhide | CC: | mcepl, mtasaka, panemade |
| Target Milestone: | --- | ||
| Target Release: | --- | ||
| Hardware: | All | ||
| OS: | Linux | ||
| Whiteboard: | |||
| Fixed In Version: | Doc Type: | Bug Fix | |
| Doc Text: | Story Points: | --- | |
| Clone Of: | Environment: | ||
| Last Closed: | 2007-01-09 15:21:31 UTC | Type: | --- |
| Regression: | --- | Mount Type: | --- |
| Documentation: | --- | CRM: | |
| Verified Versions: | Category: | --- | |
| oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |
| Cloudforms Team: | --- | Target Upstream Version: | |
| Embargoed: | |||
| Bug Depends On: | |||
| Bug Blocks: | 163779 | ||
|
Description
Matěj Cepl
2006-11-30 10:13:06 UTC
And yes this is one of the first packages I made for Fedora and the first one which I try to push into Extras. 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. 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? Unable to view SPEC and SPRM You don't have permission to access /matej/progs/rpms/vimoutliner.spec on this server. 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). 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?
(In reply to comment #6) > * Please specify all sources. I meant "Please specify URLs of all sources" I'll sponsor Matej as soon as this passes a review. Mamoru, thanks for co-reviewing this package. 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. 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). 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. 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? I should probably not use private comments in Extras bugs -- see comment 11. Sorry for that. - naming guidelines request naming the package vim-vimoutliner which seems a little bit weird but on Debian it is as well Updated spec is available on http://www.ceplovi.cz/matej/progs/rpms/vim-vimoutliner.spec, SRPM on http://www.ceplovi.cz/matej/progs/rpms/vim-vimoutliner-0.3.4-8.src.rpm, and RPM on http://www.ceplovi.cz/matej/progs/rpms/vim-vimoutliner-0.3.4-8.noarch.rpm (small changes related to change of name). 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. 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 (Changing summary for traceability) |