Bug 691635 - Review Request: prepaid-manager-applet - An applet for the GNOME Desktop for GSM mobile prepaid SIM cards
Summary: Review Request: prepaid-manager-applet - An applet for the GNOME Desktop for ...
Keywords:
Status: CLOSED DUPLICATE of bug 848551
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Nobody's working on this, feel free to take it
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On: 710411
Blocks:
TreeView+ depends on / blocked
 
Reported: 2011-03-29 05:20 UTC by Ankur Sinha (FranciscoD)
Modified: 2012-08-31 18:38 UTC (History)
8 users (show)

Fixed In Version:
Clone Of:
Environment:
Last Closed: 2012-08-15 21:03:41 UTC
Type: ---
Embargoed:


Attachments (Terms of Use)

Description Ankur Sinha (FranciscoD) 2011-03-29 05:20:34 UTC
Spec URL: http://ankursinha.fedorapeople.org/ppm/ppm.spec
SRPM URL: http://ankursinha.fedorapeople.org/ppm/ppm-20110323-1.fc16.src.rpm
Description: 
prepaid-manager-applet aims to ease the handling of mobile internet
connections using GSM mobile prepaid cards on the GNOME Desktop.
Such a SIM card can either be in a mobile phone used as a modem,
a usb 3g module (usb surf stick) or used by the built in 3G
chip set in your laptop/net book.

* It allows you to check the current balance and to top up the credit.
* It uses ModemManager to talk to your GSM chip set.

Comment 1 Ankur Sinha (FranciscoD) 2011-03-29 05:21:21 UTC
[ankur@ankur SRPMS]$ rpmlint ppm-20110323-1.fc14.src.rpm ~/rpmbuild/SPECS/ppm.spec /var/lib/mock/fedora-rawhide-i386/result/*.rpm
ppm.src: W: spelling-error %description -l en_US usb -> sub, us, sb
ppm.src: W: invalid-url Source0: ppm-20110323.tar
/home/ankur/rpmbuild/SPECS/ppm.spec: W: invalid-url Source0: ppm-20110323.tar
ppm.noarch: W: spelling-error %description -l en_US usb -> sub, us, sb
ppm.noarch: W: no-manual-page-for-binary prepaid-manager-applet
ppm.src: W: spelling-error %description -l en_US usb -> sub, us, sb
ppm.src: W: invalid-url Source0: ppm-20110323.tar
3 packages and 1 specfiles checked; 0 errors, 7 warnings.

Comment 2 Elad Alfassa 2011-03-29 07:10:43 UTC
I'll do an unofficial review.

Comment 3 Elad Alfassa 2011-03-29 07:47:24 UTC
Unofficial review:

+ = OK
- = NA
? = issue

+ Package meets naming guidelines
+ Spec file matches base package name.
? Spec has consistent macro usage.
? Meets Packaging Guidelines.
+ License
+ License field in spec matches
? License file included in package
+ Spec in American English
+ Spec is legible.
- Sources match upstream md5sum:

- Package needs ExcludeArch
? BuildRequires correct
+ Spec handles locales/find_lang
- Package is relocatable and has a reason to be.
+ Package has %defattr and permissions on files is good.
+ Package has a correct %clean section.
- Package has correct buildroot
%{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n)
+ Package is code or permissible content.
- Doc subpackage needed/used.
+ Packages %doc files don't affect runtime.

- Headers/static libs in -devel subpackage.
- Spec has needed ldconfig in post and postun
- .pc files in -devel subpackage/requires pkgconfig
- .so files in -devel subpackage.
- -devel package Requires: %{name} = %{version}-%{release}
- .la files are removed.

+ Package is a GUI app and has a .desktop file

+ Package compiles and builds on at least one arch.
+ Package has no duplicate files in %files.
+ Package doesn't own any directories other packages own.
- Package owns all the directories it creates.
- No rpmlint output.
? final provides and requires are sane:


SHOULD Items:

- Should build in mock.
- Should build on all supported archs
- Should function as described.
- Should have sane scriptlets.
- Should have subpackages require base package with fully versioned depend.
+ Should have dist tag
- Should package latest version

Issues:

1. I'm not sure about the directory in which you located the py and pyc files (seems wrong to me). Please read https://fedoraproject.org/wiki/Packaging:Python for more information.
2. Please ask upstream to include license in the git repository (and tarballs, when upstream releases those) so that you can include the license file in your package.
3. Missing build dependency: pygtk2
4. Missing run-time dependencies: mobile-broadband-provider-info, ModemManager
5. You can use %{buildroot} or $RPM_BUILD_ROOT, but you CAN NOT not both in the same spec. Read https://fedoraproject.org/wiki/Packaging/Guidelines#Using_.25.7Bbuildroot.7D_and_.25.7Boptflags.7D_vs_.24RPM_BUILD_ROOT_and_.24RPM_OPT_FLAGS for more information.

Warnings:
1. Clean section is not required for Fedora 13 and above. 

Please fix these issues, and update the spec and SRPM accordingly.

Comment 4 Ankur Sinha (FranciscoD) 2011-03-29 08:29:06 UTC
Thank you for the review.

(In reply to comment #3)
> Unofficial review:
> 
> + = OK
> - = NA
> ? = issue
> 
> + Package meets naming guidelines
> + Spec file matches base package name.
> ? Spec has consistent macro usage.
> ? Meets Packaging Guidelines.
> + License
> + License field in spec matches
> ? License file included in package
> + Spec in American English
> + Spec is legible.
> - Sources match upstream md5sum:
> 
> - Package needs ExcludeArch
> ? BuildRequires correct
> + Spec handles locales/find_lang
> - Package is relocatable and has a reason to be.
> + Package has %defattr and permissions on files is good.
> + Package has a correct %clean section.
> - Package has correct buildroot
> %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n)
> + Package is code or permissible content.
> - Doc subpackage needed/used.
> + Packages %doc files don't affect runtime.
> 
> - Headers/static libs in -devel subpackage.
> - Spec has needed ldconfig in post and postun
> - .pc files in -devel subpackage/requires pkgconfig
> - .so files in -devel subpackage.
> - -devel package Requires: %{name} = %{version}-%{release}
> - .la files are removed.
> 
> + Package is a GUI app and has a .desktop file
> 
> + Package compiles and builds on at least one arch.
> + Package has no duplicate files in %files.
> + Package doesn't own any directories other packages own.
> - Package owns all the directories it creates.
> - No rpmlint output.
> ? final provides and requires are sane:
> 
> 
> SHOULD Items:
> 
> - Should build in mock.
> - Should build on all supported archs
> - Should function as described.
> - Should have sane scriptlets.
> - Should have subpackages require base package with fully versioned depend.
> + Should have dist tag
> - Should package latest version
> 
> Issues:
> 
> 1. I'm not sure about the directory in which you located the py and pyc files
> (seems wrong to me). Please read
> https://fedoraproject.org/wiki/Packaging:Python for more information.

I checked up on this already. I'm not sure either. As of now, the files are placed where the make script puts them. I'll contact upstream to confirm.

> 2. Please ask upstream to include license in the git repository (and tarballs,
> when upstream releases those) so that you can include the license file in your
> package.

I'll do that and include a license as a SOURCE: in the meantime.

> 3. Missing build dependency: pygtk2

Weird, it built in mock correctly.

> 4. Missing run-time dependencies: mobile-broadband-provider-info, ModemManager

Do I need to specify explicit requires? 

> 5. You can use %{buildroot} or $RPM_BUILD_ROOT, but you CAN NOT not both in the
> same spec. Read
> https://fedoraproject.org/wiki/Packaging/Guidelines#Using_.25.7Bbuildroot.7D_and_.25.7Boptflags.7D_vs_.24RPM_BUILD_ROOT_and_.24RPM_OPT_FLAGS
> for more information.

Corrected.

> 
> Warnings:
> 1. Clean section is not required for Fedora 13 and above. 

Removed. 

> 
> Please fix these issues, and update the spec and SRPM accordingly.

Thanks,
Ankur

Comment 5 Elad Alfassa 2011-03-29 08:36:56 UTC
(In reply to comment #4)
> Thank you for the review.
> 
> > 1. I'm not sure about the directory in which you located the py and pyc files
> > (seems wrong to me). Please read
> > https://fedoraproject.org/wiki/Packaging:Python for more information.
> 
> I checked up on this already. I'm not sure either. As of now, the files are
> placed where the make script puts them. I'll contact upstream to confirm.
> 
Well I think it's not an upstream issue but rather a packaging issue. Please try to install it in a normal location for python files (written in the guidelines) and if it works, then use it. If not, report a bug to the upstream.
> > 2. Please ask upstream to include license in the git repository (and tarballs,
> > when upstream releases those) so that you can include the license file in your
> > package.
> 
> I'll do that and include a license as a SOURCE: in the meantime.
Sounds fine.
> 
> > 3. Missing build dependency: pygtk2
> 
> Weird, it built in mock correctly.
Upstream list it as a dependency. 
> 
> > 4. Missing run-time dependencies: mobile-broadband-provider-info, ModemManager
> 
> Do I need to specify explicit requires? 
I think so.
> 
> > 5. You can use %{buildroot} or $RPM_BUILD_ROOT, but you CAN NOT not both in the
> > same spec. Read
> > https://fedoraproject.org/wiki/Packaging/Guidelines#Using_.25.7Bbuildroot.7D_and_.25.7Boptflags.7D_vs_.24RPM_BUILD_ROOT_and_.24RPM_OPT_FLAGS
> > for more information.
> 
> Corrected.
> 
> > 
> > Warnings:
> > 1. Clean section is not required for Fedora 13 and above. 
> 
> Removed. 
> 
> > 
> > Please fix these issues, and update the spec and SRPM accordingly.
> 
> Thanks,
> Ankur
Please update the spec and SRPM for any change you make, and give a new link for the SRPM. (the spec should also be updated, but with the same link).

Comment 6 Ankur Sinha (FranciscoD) 2011-03-29 09:55:31 UTC
(In reply to comment #5)
> (In reply to comment #4)
> > Thank you for the review.
> > 
> > 
> > > 3. Missing build dependency: pygtk2
> > 
> > Weird, it built in mock correctly.
> Upstream list it as a dependency. 
> > 
> > > 4. Missing run-time dependencies: mobile-broadband-provider-info, ModemManager
> > 
> > Do I need to specify explicit requires? 
> I think so.
> > 

These are *not* build deps, they are runtime deps which rpm will figure out on its own. They do not need to be specified IMO.

Ankur

Comment 7 Elad Alfassa 2011-03-29 10:00:35 UTC
(In reply to comment #6)
> (In reply to comment #5)
> > (In reply to comment #4)
> > > Thank you for the review.
> > > 
> > > 
> > > > 3. Missing build dependency: pygtk2
> > > 
> > > Weird, it built in mock correctly.
> > Upstream list it as a dependency. 
> > > 
> > > > 4. Missing run-time dependencies: mobile-broadband-provider-info, ModemManager
> > > 
> > > Do I need to specify explicit requires? 
> > I think so.
> > > 
> 
> These are *not* build deps, they are runtime deps which rpm will figure out on
> its own. They do not need to be specified IMO.
> 
> Ankur
[elad@E-Desktop noarch]$ rpm -qp --requires ppm-20110323-1.fc15.noarch.rpm/bin/sh  
/usr/bin/python  
rpmlib(CompressedFileNames) <= 3.0.4-1
rpmlib(FileDigests) <= 4.6.0-1
rpmlib(PartialHardlinkSets) <= 4.0.4-1
rpmlib(PayloadFilesHavePrefix) <= 4.0-1
rpmlib(PayloadIsXz) <= 5.2-1

Doesn't seems that rpm figures it out automatically.

Comment 8 Carl Thompson 2011-03-30 03:31:41 UTC
I recommend that you use a clean mock each time you build to ensure requires are met.

If you reuse a mock environment without cleaning and rebuilding it you may have packages intalled in mock that are not installed in the koji or minimal install and you'll miss marking them as a required package.

The python (perl, and other interpreted languages) have an internal dependancy generator but they only generate dependancies for other language related items.

I do a final test in koji to ensure requires are met because koji always uses a clean minimal mock environment for the build.

Comment 9 Ankur Sinha (FranciscoD) 2011-03-30 04:12:23 UTC
(In reply to comment #8)
> I recommend that you use a clean mock each time you build to ensure requires
> are met.
> 
> If you reuse a mock environment without cleaning and rebuilding it you may have
> packages intalled in mock that are not installed in the koji or minimal install
> and you'll miss marking them as a required package.
> 
> The python (perl, and other interpreted languages) have an internal dependancy
> generator but they only generate dependancies for other language related items.
> 
> I do a final test in koji to ensure requires are met because koji always uses a
> clean minimal mock environment for the build.


IIRC mock cleans it's environment before doing the rebuild (I *think*).

I was thinking about this app and had the realization that gnome shell does not currently feature applets. I'm not sure if this is worth packaging if it won't work with F15 even?  

I'm going to contact upstream and see what they have to say about this, and other queries, such as the directory placements etc.


Thanks!
regards,
Ankur

Comment 10 Elad Alfassa 2011-03-30 05:58:50 UTC
Please note that the mobile-broadband-provider-info package contains xml files that ppm reads to get data about your provider. rpm is not smart enough to figure it out alone.

Comment 12 Felix Möller 2011-05-31 21:40:18 UTC
I think there is some breakage in the current upstream release.

The accountdb.py is not even in the release tarball and therefore missing from the resulting RPM ...

Nice to see the package here. Just read about it on planet-gnome.

Comment 13 Felix Möller 2011-06-03 11:01:43 UTC
upstream 0.0.1.2 fixes my issues mentioned in the last comment. 

For the package to be realy useful we need bug #710411.

Comment 14 Felix Möller 2011-08-14 16:56:15 UTC
There has been a new release of prepaid-manager http://honk.sigxcpu.org/con/GNOME_Prepaid_Manager_0_0_2.html

Comment 15 Ankur Sinha (FranciscoD) 2011-08-15 10:57:45 UTC
I've applied for co-maintainership for mobile-provider-info. Let's hope we can move this along quickly now :)

Comment 16 Kai Engert (:kaie) (inactive account) 2012-06-19 17:55:24 UTC
I was able to build and install it on a stock F16 system.

Updated spec file to use .tar.xz extension, to latest 0.0.3.1 version number, and to include icons in the package. (However, the icon is not being when running the app runs, nor shown in the applications menu, don't know why)

http://kuix.de/fedora/ppm/prepaid-manager-applet.spec
http://kuix.de/fedora/ppm/prepaid-manager-applet-0.0.3.1-1.fc16.noarch.rpm
http://kuix.de/fedora/ppm/prepaid-manager-applet-0.0.3.1-1.fc16.src.rpm

Comment 17 Kai Engert (:kaie) (inactive account) 2012-06-19 22:38:49 UTC
... the icon worked after logout and login.

Comment 18 Mario Blättermann 2012-08-12 21:27:03 UTC
@Kai, seems to be that the original review request rests in piece for a long time. If you want to become the package maintainer, file a new review request and mark this one as a duplicate.

@Ankur, I hope you agree...?

Comment 19 Ankur Sinha (FranciscoD) 2012-08-13 09:42:14 UTC
Sure. I think mobile-provider-info has been updated too.

Comment 20 Kai Engert (:kaie) (inactive account) 2012-08-13 16:11:09 UTC
What is blocking this package review at this time?

What work is left to be done?

Comment 21 Ankur Sinha (FranciscoD) 2012-08-14 01:25:33 UTC
(In reply to comment #20)
> What is blocking this package review at this time?
> 
> What work is left to be done?

IIRC, mobile-provider-info being updated was the only blocker, which seems to now be solved.

Comment 22 Mario Blättermann 2012-08-15 19:03:22 UTC
(In reply to comment #20)
> What is blocking this package review at this time?
> 
> What work is left to be done?

As I already wrote, If you want to maintain this package, please open a new review request and mark this one as a duplicate.

Comment 23 Kai Engert (:kaie) (inactive account) 2012-08-15 19:16:23 UTC
> As I already wrote, If you want to maintain this package, please open a new
> review request and mark this one as a duplicate.


I can do that, I just don't understand why it's necessary.

Is it because FranciscoD doesn't want to be the maintainer any longer, and the package maintainer must be the person who files the bug?

Comment 24 Mario Blättermann 2012-08-15 19:21:22 UTC
(In reply to comment #23)
> Is it because FranciscoD doesn't want to be the maintainer any longer, and
> the package maintainer must be the person who files the bug?

Yes, you cannot migrate a review request to your own. Just file a new bug, and all is OK. I think I will do the review.

Comment 25 Kai Engert (:kaie) (inactive account) 2012-08-15 21:03:41 UTC

*** This bug has been marked as a duplicate of bug 848551 ***

Comment 26 Kai Engert (:kaie) (inactive account) 2012-08-31 18:38:15 UTC
If you are interested in this package,

please get it from updates-testing and give feedback/karma.
Necessary to get the package included in the regular update area.

Please see bug 848551 comment 18. Thank you!


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