| Summary: | Review Request: timeline - Displays and navigates events on a timeline | ||||||
|---|---|---|---|---|---|---|---|
| Product: | [Fedora] Fedora | Reporter: | Gwyn Ciesla <gwync> | ||||
| Component: | Package Review | Assignee: | Mamoru TASAKA <mtasaka> | ||||
| Status: | CLOSED ERRATA | QA Contact: | Fedora Extras Quality Assurance <extras-qa> | ||||
| Severity: | unspecified | Docs Contact: | |||||
| Priority: | unspecified | ||||||
| Version: | rawhide | CC: | notting, package-review | ||||
| Target Milestone: | --- | Flags: | mtasaka:
fedora-review+
gwync: fedora-cvs+ |
||||
| Target Release: | --- | ||||||
| Hardware: | Unspecified | ||||||
| OS: | Unspecified | ||||||
| Whiteboard: | |||||||
| Fixed In Version: | timeline-0.14.0-3.fc15 | Doc Type: | Bug Fix | ||||
| Doc Text: | Story Points: | --- | |||||
| Clone Of: | Environment: | ||||||
| Last Closed: | 2011-10-29 18:27:23 UTC | Type: | --- | ||||
| Regression: | --- | Mount Type: | --- | ||||
| Documentation: | --- | CRM: | |||||
| Verified Versions: | Category: | --- | |||||
| oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |||||
| Cloudforms Team: | --- | Target Upstream Version: | |||||
| Attachments: |
|
||||||
|
Description
Gwyn Ciesla
2011-10-20 13:54:30 UTC
I will take this one. By the way I appreciate it if you have some time to my review request bug 744628. Some notes
* License
- README says that this software is GPLv3 only
- icons/COPYING says that icons are under CC-BY-SA
- So I think the License tag should be "GPLv3 and CC-BY-SA".
* Unneeded specification
- If you don't intend to import this into EPEL, the following
items are no longer needed and should be removed.
- BuildRoot line
- "rm -rf $RPM_BUILD_ROOT" at the top of %install
- %clean section entirely
* BR
- Is scons really needed?
* Requires
- Usually explicitly writing "hicolor-icon-theme" is not required.
(I don't oppose to leave it though)
* Macros
- Please use macros when possible. For example /usr/share/man
should be %{_mandir}.
" Scriptlets
- Please follow:
https://fedoraproject.org/wiki/Packaging/ScriptletSnippets#Icon_Cache
(gtk-update-icon-cache oon %post moved to %posttrans)
* Documents
- "INSTALL" file is usually for people trying to build and install
a package by themselves and not needed for people using rpm
mechanism. Also please check the result of rpmlint.
* gettext .po file issue
- Check release/deb/debian/patches/fix-paths.patch and try to change
LOCALE_DIR in main/timelinelib/paths.py.
Fixed license tag. I may build for EPEL. Scons is for building translations, but I can't get find_lang to work. Fixed scriptlet. I've modified my patch for the po paths, but I thought we weren't supposed to drop things in /u/s/locale manually. About locale files
- Fedora review guideline requests that locale files must be handled
with %find_lang. This eventually requests that locale files are
under %_datadir/locale/..... . Also man 3 gettext says that the default
path for locale files is there. So unless you have some reason,
using %_datadir/locale is preferred.
- I prefer to use "msgfmt -o foo.mo foo.po" manually if scons is used
only to build gettext .mo files.
By the way, anyway would you upload (and post the URLs of) srpm
and spec file you are currently working on?
Right, but find_lang finds nothing. The current iteration: SPEC: http://zanoni.jcomserv.net/fedora/timeline/timeline.spec SRPM: http://zanoni.jcomserv.net/fedora/timeline/timeline-0.14.0-2.fc15.src.rpm I will check this later (maybe tomorrow...) Created attachment 530699 [details]
spec file to install gettext .mo files
* About gettext .mo files
- So you have to just build .mo files with gettext command and install
the created .mo files to appropriate locations. Please check the attached
spec file (and also check the lines about 80-90 of SConstruct).
* Scriptlets
- Fedora packaging guideline says "touch --no-create
%{_datadir}/icons/hicolor || : " is in %post and %postun, not in
%posttrans.
Please check the attached spec file, modify scriptlets and add the needed fixes if you want, and then upload new spec / srpm again.
SPEC: http://zanoni.jcomserv.net/fedora/timeline/timeline.spec SRPM: http://zanoni.jcomserv.net/fedora/timeline/timeline-0.14.0-3.fc15.src.rpm Beautiful, thank you! Well, * Please check scriptlets again. https://fedoraproject.org/wiki/Packaging/ScriptletSnippets#Icon_Cache touch .... should be in %post and %postun, but gtk-update-icon-cache should be in %postun and %posttrans (both were in %post and %postun, however guideline changed on 2009-03). ref: http://lists.fedoraproject.org/pipermail/devel-announce/2009-April/000404.html https://fedoraproject.org/w/index.php?title=Packaging%3AScriptletSnippets&action=historysubmit&diff=89534&oldid=72483 * Now I don't think BR: scons is needed. Please fix above when importing this package into Fedora git. ------------------------------------------------------------- This pacakge (timeline) is APPROVED by mtasaka ------------------------------------------------------------- You're right, I missed that, and I'll pull scons. Thanks for the review! New Package SCM Request ======================= Package Name: timeline Short Description: Displays and navigates events on a timeline Owners: limb Branches: f15 f16 InitialCC: Git done (by process-git-requests). timeline-0.14.0-3.fc15 has been submitted as an update for Fedora 15. https://admin.fedoraproject.org/updates/timeline-0.14.0-3.fc15 timeline-0.14.0-3.fc16 has been submitted as an update for Fedora 16. https://admin.fedoraproject.org/updates/timeline-0.14.0-3.fc16 timeline-0.14.0-3.fc16 has been pushed to the Fedora 16 testing repository. Closing. timeline-0.14.0-3.fc16 has been pushed to the Fedora 16 stable repository. timeline-0.14.0-3.fc15 has been pushed to the Fedora 15 stable repository. |