[Bug 760033] Review Request: kde-plasma-publictransport - Public Transport plasma applet
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug. https://bugzilla.redhat.com/show_bug.cgi?id=760033 Gregor Tätzner gre...@freenet.de changed: What|Removed |Added Status|NEW |CLOSED Resolution||ERRATA Last Closed||2012-02-17 13:40:55 --- Comment #18 from Gregor Tätzner gre...@freenet.de 2012-02-17 13:40:55 EST --- pushed to stable -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 760033] Review Request: kde-plasma-publictransport - Public Transport plasma applet
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug. https://bugzilla.redhat.com/show_bug.cgi?id=760033 Gregor Tätzner gre...@freenet.de changed: What|Removed |Added Flag||fedora-cvs? --- Comment #16 from Gregor Tätzner gre...@freenet.de 2012-01-21 09:30:49 EST --- Hurray, thanks Brendan. I have already contacted upstream. New Package SCM Request === Package Name: kde-plasma-publictransport Short Description: Public Transport plasma applet Owners: brummbq Branches: f15 f16 InitialCC: -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 760033] Review Request: kde-plasma-publictransport - Public Transport plasma applet
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug. https://bugzilla.redhat.com/show_bug.cgi?id=760033 --- Comment #17 from Jon Ciesla limburg...@gmail.com 2012-01-21 14:33:54 EST --- Git done (by process-git-requests). -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 760033] Review Request: kde-plasma-publictransport - Public Transport plasma applet
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug. https://bugzilla.redhat.com/show_bug.cgi?id=760033 --- Comment #14 from Gregor Tätzner gre...@freenet.de 2012-01-20 06:15:08 EST --- Cool Spec URL: http://brummbq.fedorapeople.org/kde-plasma-publictransport.spec SRPM Url: http://brummbq.fedorapeople.org/kde-plasma-publictransport-0.10-0.4.20111204git.fc16.src.rpm -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 760033] Review Request: kde-plasma-publictransport - Public Transport plasma applet
Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=760033
Brendan Jones brendan.jones...@gmail.com changed:
What|Removed |Added
Flag|fedora-review? |fedora-review+
--- Comment #15 from Brendan Jones brendan.jones...@gmail.com 2012-01-20
18:05:34 EST ---
Looks good Greg, just the #incomplete comment and a question on the license.
Each sub-dir has its own COPYING file, I noticed you've %doc the file from
applet/ directory. (I've checked that its the same file in every directory) You
could ask upstream to drop the COPYING file in the root directory (next to the
HINTS_FOR_PACKAGE_MAINTAINERS file, seeing as they're being so helpful)
This package is APPROVED
Required
+ - OK
- - N/A
X - attention
? - comment please
[+] named according to the Package Naming Guidelines
[+] The spec file name must match the base package %{name}, in the format
%{name}.spec
[X] Meet the Packaging Guidelines
# extraneous comment - I think I understand why - see below
[+] Be licensed with a Fedora approved license and meet the Licensing
Guidelines
[+] The License field in the package spec file must match the actual license
[+] License file must be included in %doc
[+] The spec file must be written in American English
[+] The spec file for the package MUST be legible
[+] The sources used to build the package must match the upstream source
# git
[+] Successfully compile and build into binary rpms on at least one primary
architecture
[+] Proper use of ExcludeArch
[+] All build dependencies must be listed in BuildRequires
[+] The spec file MUST handle locales properly
[+] Shared library files (not just symlinks) in any of the dynamic linker's
default paths, must call ldconfig in %post and %postun
[+] Packages must NOT bundle copies of system libraries
[-] If the package is designed to be relocatable, the packager must state this
fact in the request for review, along with the rationalization for relocation
of that specific package
[+] A package must own all directories that it creates
directories under this
[+] A Fedora package must not list a file more than once in the spec file's
%files listings
[+] Permissions on files must be set properly.
[+] Each package must consistently use macros
[+] The package must contain code, or permissable content
[-] Large documentation files must go in a -doc subpackage
[+] If a package includes something as %doc, it must not affect the runtime of
the application
[+] Header files must be in a -devel package
[-] Static libraries must be in a -static package
[+] library files that end in .so (without suffix) must go in a -devel package
You have separated the *_debug.so files out into a separate package?
[+] devel packages must require the base package using a fully versioned
dependency
[+] Packages must NOT contain any .la libtool archives
[+] GUI apps must include a %{name}.desktop file, properly installed with
desktop-file-install in the %install section
[+] Packages must not own files or directories already owned by other packages
[+] All filenames in rpm packages must be valid UTF-8
[+] Packaged using KDE macros
Should Items
[-] the packager SHOULD query upstream for any missing license text files to
include it
[-] Non-English language support for description and summary sections in the
package spec if available
[+] The reviewer should test that the package builds in mock
[-] The package should compile and build into binary rpms on all supported
architectures
[+] The reviewer should test that the package functions as described
[+] If scriptlets are used, those scriptlets must be sane
[+] Usually, subpackages other than devel should require the base package using
a fully versioned dependency
[-] The placement of pkgconfig(.pc) should usually be placed in a -devel pkg
[-] If the package has file dependencies outside of /etc, /bin, /sbin,
/usr/bin, or /usr/sbin consider requiring the package which provides the file
instead of the file itself
[-] Should contain man pages for binaries/scripts
--
Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are on the CC list for the bug.
___
package-review mailing list
package-review@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 760033] Review Request: kde-plasma-publictransport - Public Transport plasma applet
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug. https://bugzilla.redhat.com/show_bug.cgi?id=760033 --- Comment #13 from Brendan Jones brendan.jones...@gmail.com 2012-01-19 16:20:26 EST --- Hi Greg, you'll need it on %posttrans as well. I'll try and finish this for you tomorrow. -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 760033] Review Request: kde-plasma-publictransport - Public Transport plasma applet
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug. https://bugzilla.redhat.com/show_bug.cgi?id=760033 --- Comment #12 from Gregor Tätzner gre...@freenet.de 2012-01-18 16:32:33 EST --- Doh...now I have got it, thanks ;) Spec URL: http://brummbq.fedorapeople.org/kde-plasma-publictransport.spec SRPM Url: http://brummbq.fedorapeople.org/kde-plasma-publictransport-0.10-0.3.20111204git.fc16.src.rpm -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 760033] Review Request: kde-plasma-publictransport - Public Transport plasma applet
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug. https://bugzilla.redhat.com/show_bug.cgi?id=760033 --- Comment #11 from Brendan Jones brendan.jones...@gmail.com 2012-01-17 13:09:55 EST --- I haven't really looked yet, but what Kevin is saying is that you need to put your icons scriplets under the correct subpackage. ie in %post(un) libs . . . . /usr/bin/ldconfig Make sure the ldconfig call is last. As they stand at the moment your icon scriplets apply to the main package. -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 760033] Review Request: kde-plasma-publictransport - Public Transport plasma applet
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug. https://bugzilla.redhat.com/show_bug.cgi?id=760033 Brendan Jones brendan.jones...@gmail.com changed: What|Removed |Added CC||brendan.jones...@gmail.com AssignedTo|nob...@fedoraproject.org|brendan.jones...@gmail.com Flag||fedora-review? --- Comment #10 from Brendan Jones brendan.jones...@gmail.com 2012-01-16 12:38:59 EST --- I can take this review. -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 760033] Review Request: kde-plasma-publictransport - Public Transport plasma applet
Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=760033
--- Comment #9 from Gregor Tätzner gre...@freenet.de 2011-12-22 10:23:06 EST
---
(In reply to comment #8)
The problem is, in libs subpackage are also icons for timetablemate
included.
This means I can't move them all together into the main package because
timetable doesn't need the rest of the main package.
Well, right now it does because you have a Requires…
To achieve what you want, please:
* remove the Requires: %{name} = %{version}-%{release} from -libs and
check
* make the icon scriptlets apply to the -libs subpackage where the icons
actually are. (You'll have to remove the -p /sbin/ldconfig and write
/sbin/ldconfig as a scriptlet line instead, so that the other stuff can be
added.)
What do you mean exactly by make the icon scriptlets apply to the -libs
subpackage I'm a little bit confused.
Another point: Atm my package is missing some doc files (changelogs from
various plasmoids in the main package) How can I include them? They have all
the same file name.
--
Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are on the CC list for the bug.
___
package-review mailing list
package-review@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 760033] Review Request: kde-plasma-publictransport - Public Transport plasma applet
Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=760033
--- Comment #7 from Gregor Tätzner gre...@freenet.de 2011-12-21 06:18:19 EST
---
Spec URL: http://brummbq.fedorapeople.org/kde-plasma-publictransport.spec
SRPM Url:
http://brummbq.fedorapeople.org/kde-plasma-publictransport-0.10-0.2.20111204git.fc16.src.rpm
Thank you for your feedback.
(In reply to comment #6)
Release:%{snapshot}%{?dist}.1
should be:
Release:0.1.%{snapshot}%{?dist}
The next builds should then be 0.2.%{snapshot}%{?dist},
0.3.%{snapshot}%{?dist}
etc.
done
BuildRequires are global, so I'd list them all together at the beginning (but
putting them where you put them works, too).
done
#remove executable bit
chmod 644
%{buildroot}/%{_kde4_datadir}/applications/kde4/timetablemate.desktop
Not sure about that one… We don't normally do this, i.e. we keep the +x bit
upstream sets, but some non-KDE folks want us to do what you did everywhere.
In
practice, it will work either way because KDE Plasma does not require the +x
bit in the standard prefix.
So, I'll leave it as it is. Otherwise rpmlint is complaining.
You put the icon scriptlets into the main package and the actual icons
into -libs. They should be in the same package. (IMHO in the main package.
Those icons shouldn't be multilibbed.)
The problem is, in libs subpackage are also icons for timetablemate included.
This means I can't move them all together into the main package because
timetable doesn't need the rest of the main package. Well, and I was too lazy
to split up the icons suitably :) (Probably I would make horrible mistakes)
--
Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are on the CC list for the bug.
___
package-review mailing list
package-review@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 760033] Review Request: kde-plasma-publictransport - Public Transport plasma applet
Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=760033
--- Comment #8 from Kevin Kofler ke...@tigcc.ticalc.org 2011-12-21 10:12:20
EST ---
The problem is, in libs subpackage are also icons for timetablemate included.
This means I can't move them all together into the main package because
timetable doesn't need the rest of the main package.
Well, right now it does because you have a Requires…
To achieve what you want, please:
* remove the Requires: %{name} = %{version}-%{release} from -libs and
* make the icon scriptlets apply to the -libs subpackage where the icons
actually are. (You'll have to remove the -p /sbin/ldconfig and write
/sbin/ldconfig as a scriptlet line instead, so that the other stuff can be
added.)
--
Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are on the CC list for the bug.
___
package-review mailing list
package-review@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 760033] Review Request: kde-plasma-publictransport - Public Transport plasma applet
Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=760033
--- Comment #6 from Kevin Kofler ke...@tigcc.ticalc.org 2011-12-20 19:15:43
EST ---
IMHO, the subpackages are fine as is, the openSUSE version is excessively
split.
Some comments (not a complete review yet):
Release:%{snapshot}%{?dist}.1
should be:
Release:0.1.%{snapshot}%{?dist}
The next builds should then be 0.2.%{snapshot}%{?dist}, 0.3.%{snapshot}%{?dist}
etc.
BuildRequires are global, so I'd list them all together at the beginning (but
putting them where you put them works, too).
#remove executable bit
chmod 644
%{buildroot}/%{_kde4_datadir}/applications/kde4/timetablemate.desktop
Not sure about that one… We don't normally do this, i.e. we keep the +x bit
upstream sets, but some non-KDE folks want us to do what you did everywhere. In
practice, it will work either way because KDE Plasma does not require the +x
bit in the standard prefix.
You put the icon scriptlets into the main package and the actual icons
into -libs. They should be in the same package. (IMHO in the main package.
Those icons shouldn't be multilibbed.)
That's all I can spot at first glance, I haven't done a complete systematic
review yet though.
--
Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are on the CC list for the bug.
___
package-review mailing list
package-review@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 760033] Review Request: kde-plasma-publictransport - Public Transport plasma applet
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug. https://bugzilla.redhat.com/show_bug.cgi?id=760033 Gregor Tätzner gre...@freenet.de changed: What|Removed |Added Blocks||656997(kde-reviews) -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 760033] Review Request: kde-plasma-publictransport - Public Transport plasma applet
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug. https://bugzilla.redhat.com/show_bug.cgi?id=760033 --- Comment #5 from Gregor Tätzner gre...@freenet.de 2011-12-13 13:11:55 EST --- Spec URL: http://brummbq.fedorapeople.org/kde-plasma-publictransport.spec SRPM Url: http://brummbq.fedorapeople.org/kde-plasma-publictransport-0.10-20111204git.fc16.1.src.rpm minimal changes. Still not sure about package splitting. The guys at suse have one subpackage for every plasmoid/krunner. I don't like that...it feels too scattered. https://build.opensuse.org/package/view_file?file=plasmoid-publictransport.specpackage=plasmoid-publictransportproject=KDE%3AExtrarev=306504949ccff771fe2c41b9f1cd2571 -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 760033] Review Request: kde-plasma-publictransport - Public Transport plasma applet
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug. https://bugzilla.redhat.com/show_bug.cgi?id=760033 --- Comment #1 from Gregor Tätzner gre...@freenet.de 2011-12-05 05:07:14 EST --- source obtained via: git archive --format=tar --remote=git://anongit.kde.org/publictransport HEAD | gzip publictransport_20111204git.tar.gz -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 760033] Review Request: kde-plasma-publictransport - Public Transport plasma applet
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug. https://bugzilla.redhat.com/show_bug.cgi?id=760033 --- Comment #2 from Gregor Tätzner gre...@freenet.de 2011-12-05 05:16:11 EST --- sry guys wrong spec in the first comment Spec URL: http://brummbq.fedorapeople.org/kde-plasma-publictransport.spec SRPM Url: http://brummbq.fedorapeople.org/kde-plasma-publictransport-0.10-20111204git.fc16.0.src.rpm -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 760033] Review Request: kde-plasma-publictransport - Public Transport plasma applet
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug. https://bugzilla.redhat.com/show_bug.cgi?id=760033 Kevin Kofler ke...@tigcc.ticalc.org changed: What|Removed |Added CC||ke...@tigcc.ticalc.org --- Comment #3 from Kevin Kofler ke...@tigcc.ticalc.org 2011-12-05 12:32:54 EST --- Great, I'd really like to see the publictransport plasmoid in Fedora! Are you already sponsored? -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 760033] Review Request: kde-plasma-publictransport - Public Transport plasma applet
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug. https://bugzilla.redhat.com/show_bug.cgi?id=760033 --- Comment #4 from Gregor Tätzner gre...@freenet.de 2011-12-05 12:53:55 EST --- yes! I'm the maintainer for semantik, very easy :D -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
