Bug 580422
| Summary: | Review Request: libdesktop-agnostic - Provides an extensible configuration API | ||
|---|---|---|---|
| Product: | [Fedora] Fedora | Reporter: | leigh scott <leigh123linux> |
| Component: | Package Review | Assignee: | Rex Dieter <rdieter> |
| Status: | CLOSED RAWHIDE | QA Contact: | Fedora Extras Quality Assurance <extras-qa> |
| Severity: | medium | Docs Contact: | |
| Priority: | medium | ||
| Version: | rawhide | CC: | fedora-package-review, leigh123linux, notting, rdieter |
| Target Milestone: | --- | Flags: | rdieter:
fedora-review+
kevin: fedora-cvs+ |
| Target Release: | --- | ||
| Hardware: | All | ||
| OS: | Linux | ||
| Whiteboard: | |||
| Fixed In Version: | Doc Type: | Bug Fix | |
| Doc Text: | Story Points: | --- | |
| Clone Of: | Environment: | ||
| Last Closed: | 2010-08-09 21:57:19 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: | |||
|
Description
leigh scott
2010-04-08 08:52:14 UTC
New release Spec URL: http://fedoraforum.org/leigh123linux/review/1/libdesktop-agnostic/libdesktop-agnostic.spec SRPM URL: http://fedoraforum.org/leigh123linux/review/1/libdesktop-agnostic/libdesktop-agnostic-0.3.90-1.fc13.src.rpm (In reply to comment #1) Wrong URL Spec URL: http://fedoraforum.org/leigh123linux/review/libdesktop-agnostic/1/libdesktop-agnostic.spec SRPM URL: http://fedoraforum.org/leigh123linux/review/libdesktop-agnostic/1/libdesktop-agnostic-0.3.90-1.fc13.src.rpm I'll take a look. (In reply to comment #3) > I'll take a look. Thanks, I have updated it again ( I have removed some sub-packages ) Spec URL: http://fedoraforum.org/leigh123linux/review/libdesktop-agnostic/2/libdesktop-agnostic.spec SRPM URL: http://fedoraforum.org/leigh123linux/review/libdesktop-agnostic/2/libdesktop-agnostic-0.3.90-3.fc13.src.rpm Initial comments:
1. scriptlets. the only (sub)pkg that appears to contain shared libraries is the main one, the others don't need any /sbin/ldconfig scriptlets
2. the main package should own
%{_libdir}/desktop-agnostic/modules/
3. dup'd copies of
%doc COPYING COPYING.GPL-2
in subpkgs aren't needed, at least those that already
Requires: %{name} ...
since these are already included there.
Scratch build: http://koji.fedoraproject.org/koji/taskinfo?taskID=2116314 4. in %build , use ./waf -v build instead. It leads to the next issue 5. using increased verbosity, it's clear $RPM_OPT_FLAGS arent being used here. For example, [ 51/119] cc: build/default/libdesktop-agnostic/ui-icon-chooser-dialog.c -> build/default/libdesktop-agnostic/ui-icon-chooser-dialog_5.o 20:55:13 runner system command -> ['/usr/lib64/ccache/gcc', '-ggdb', '-D_GNU_SOURCE', '-DHAVE_BUILD_CONFIG_H', '-fPIC', '-DPIC', '-Idefault', '-I..', '-I/home/rdieter1/cvs.fedoraproject.org/work/libagnostic/libdesktop-agnostic-0.3.90/build/default/libdesktop-agnostic', '-I/home/rdieter1/cvs.fedoraproject.org/work/libagnostic/libdesktop-agnostic-0.3.90/libdesktop-agnostic', '-I/home/rdieter1/cvs.fedoraproject.org/work/libagnostic/libdesktop-agnostic-0.3.90/build/default/', '-I/home/rdieter1/cvs.fedoraproject.org/work/libagnostic/libdesktop-agnostic-0.3.90', '-I/usr/include/glib-2.0', '-I/usr/lib64/glib-2.0/include', '-I/usr/include/gtk-2.0', '-I/usr/lib64/gtk-2.0/include', '-I/usr/include/pango-1.0', '-I/usr/include/cairo', '-I/usr/include/pixman-1', '-I/usr/include/freetype2', '-I/usr/include/libpng12', '-I/usr/include/atk-1.0', 'default/libdesktop-agnostic/ui-icon-chooser-dialog.c', '-c', '-o', 'default/libdesktop-agnostic/ui-icon-chooser-dialog_5.o'] $ rpmlint *.rpm x86_64/*rpm libdesktop-agnostic.src:57: W: mixed-use-of-spaces-and-tabs (spaces: line 6, tab: line 57) gir1.0-desktopagnostic.x86_64: W: spelling-error Summary(en_US) GObject -> G Object, Object, Subject gir1.0-desktopagnostic.x86_64: W: spelling-error Summary(en_US) libdesktop -> lib desktop, lib-desktop, desktop gir1.0-desktopagnostic.x86_64: W: only-non-binary-in-usr-lib libdesktop-agnostic.x86_64: W: non-conffile-in-etc /etc/xdg/libdesktop-agnostic/desktop-agnostic.ini python-desktop-agnostic.x86_64: W: spelling-error Summary(en_US) libdesktop -> lib desktop, lib-desktop, desktop 10 packages and 0 specfiles checked; 0 errors, 6 warnings. 6. SHOULD use %config /etc/xdg/libdesktop-agnostic/desktop-agnostic.ini (unless there's some good reason not to). Thanks, I have updated it with the recommended changes Spec URL: http://fedoraforum.org/leigh123linux/review/libdesktop-agnostic/3/libdesktop-agnostic.spec SRPM URL: http://fedoraforum.org/leigh123linux/review/libdesktop-agnostic/3/libdesktop-agnostic-0.3.90-4.fc13.src.rpm Some more minor changes Spec URL: http://fedoraforum.org/leigh123linux/review/libdesktop-agnostic/4/libdesktop-agnostic.spec SRPM URL: http://fedoraforum.org/leigh123linux/review/libdesktop-agnostic/4/libdesktop-agnostic-0.3.90-5.fc13.src.rpm Some more changes Spec URL: http://fedoraforum.org/leigh123linux/review/libdesktop-agnostic/5/libdesktop-agnostic.spec SRPM URL: http://fedoraforum.org/leigh123linux/review/libdesktop-agnostic/5/libdesktop-agnostic-0.3.90-6.fc13.src.rpm
I like the simplification of the packaging and subpkgs.
wrt item 6, rpmlint notes now,
libdesktop-agnostic.x86_64: W: conffile-without-noreplace-flag /etc/xdg/libdesktop-agnostic/desktop-agnostic.ini
so maybe better to use
%config(noreplace)
so that end-user modifications don't get lost on upgrades (unless there's good reason to not preserve them).
nitty gritty,
naming: ok
macros: ok
licensing: ok
sources: ok
$ md5sum *.gz
67141440b7fa77e2d704e7368c067212 libdesktop-agnostic-0.3.90.tar.gz
dir ownership: ok (well, looks like ownership of %{_libdir}/desktop-agnostic/modules , got lost, re-add that to the main pkg please)
static libs: ok
libtool archives: ok
APPROVED. (I'll trust you to address the dir ownership and %config items above prior to releasing anything).
New Package CVS Request ======================= Package Name: libdesktop-agnostic Short Provides an extensible configuration API Owners: leigh123linux Branches: F-11 F-12 F-13 InitialCC: leigh123linux (In reply to comment #12) > > > APPROVED. (I'll trust you to address the dir ownership and %config items above > prior to releasing anything). Thank you for the review, I will address the last two points when I commit to CVS. Your "Short Description:" line seems malformed and is causing our cvs processing script to complain. ;) Can you please add a new cvs request with that corrected? New Package CVS Request ======================= Package Name: libdesktop-agnostic Short Description: Provides an extensible configuration API Owners: leigh123linux Branches: F-11 F-12 F-13 InitialCC: leigh123linux CVS done (by process-cvs-requests.py). |