Bug 532779 - Review Request: gtraffic - Simple traffic usage counter for mobile broadband connections
Summary: Review Request: gtraffic - Simple traffic usage counter for mobile broadband ...
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Christoph Wickert
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2009-11-03 19:43 UTC by Thomas Spura
Modified: 2009-11-20 05:13 UTC (History)
3 users (show)

Fixed In Version: 1.01-4.fc11
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2009-11-20 05:13:44 UTC
Type: ---
cwickert: fedora-review+
kevin: fedora-cvs+


Attachments (Terms of Use)

Description Thomas Spura 2009-11-03 19:43:28 UTC
Spec URL: http://tomspur.fedorapeople.org/review/gtraffic.spec
SRPM URL: http://tomspur.fedorapeople.org/review/gtraffic-1.01-2.fc11.src.rpm

Description:
gtraffic is a very quick and simple network traffic counter. It has the ability
to manually change the used traffic, for example if used between two computers.
It features a reset, so the usage can be reset at required intervals.

This utility will only work with network manager and a mobile broadband 
connection, otherwise no data will be counted.

______________________

$ rpmlint gtraffic.spec gtraffic-1.01-2.fc11.src.rpm noarch/gtraffic-1.01-2.fc11.noarch.rpm 
2 packages and 1 specfiles checked; 0 errors, 0 warnings.

______________________

Builds in Koji: http://koji.fedoraproject.org/koji/taskinfo?taskID=1786420

Comment 1 Christoph Wickert 2009-11-04 00:44:31 UTC
REVIEW for 7f6282bd6420f2ac611fd5a33e7b28a1  gtraffic-1.01-2.fc11.src.rpm

OK - MUST: rpmlint silent
OK - MUST: named according to the Package Naming Guidelines
OK - MUST: spec file name matches the base package %{name}
OK - MUST: package meets the Packaging Guidelines
OK - MUST: Fedora approved license and meets the Licensing Guidelines: GPLv3+
OK - MUST: License field in spec file matches the actual license
N/A - MUST: license file included in %doc
OK - MUST: spec is in American English
OK - MUST: spec is legible
OK - MUST: sources match the upstream source by MD5 4e3b45e8bbc7c111ee277944da754f80
OK - MUST: successfully compiles and builds into binary rpms on all arches
OK - MUST: all build dependencies are listed in BuildRequires
N/A - MUST: handles locales properly with %find_lang
N/A - MUST: Every binary RPM package (or subpackage) which stores shared library files in any of the dynamic linker's default paths, must call ldconfig in %post and %postun
N/A - MUST: Not designed to be relocatable (none)
OK - MUST: owns all directories that it creates
OK - MUST: no duplicate files in the %files listing
OK - MUST: Permissions on files are set properly, includes %defattr(...)
OK - MUST: package has a %clean section, which contains rm -rf %{buildroot}
OK - MUST: consistently uses macros
OK - MUST: package contains code
N/A - MUST: Large documentation files should go in a -doc subpackage
OK - MUST: Files included as %doc do not affect the runtime of the application
N/A - MUST: Header files must be in a -devel package
N/A - MUST: Static libraries must be in a -static package
N/A - MUST: Packages containing pkgconfig(.pc) files must 'Requires: pkgconfig'
N/A - MUST: If a package contains library files with a suffix, then library files that end in .so must go in a -devel package
N/A - MUST: devel packages must require the base package using a fully versioned dependency
OK - MUST: The package does not contain any .la libtool archives
OK - MUST: The package contains a GUI application and includes a %{name}.desktop file, and that file is properly installed with desktop-file-install in the %install section.
OK - MUST: packages does not own files or directories already owned by other packages.
OK - MUST: at the beginning of %install, the package runs rm -rf %{buildroot}
OK - MUST: all filenames valid UTF-8


SHOULD Items:
N/A - SHOULD: Source package includes license text(s) as a separate file
N/A - SHOULD: The description and summary sections in the package spec file should contain translations for supported Non-English languages
OK - SHOULD: builds in mock
OK - SHOULD: compiles and builds into binary rpms on all supported architectures
OK - SHOULD: functions as described
N/A - SHOULD: If scriptlets are used, those scriptlets must be sane
N/A - SHOULD: Usually, subpackages other than devel should require the base package using a fully versioned dependency
N/A - SHOULD: pkgconfig(.pc) files should be placed in a -devel pkg
N/A - SHOULD: 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.


Other items:
OK - latest stable version
OK - SourceURL valid
N/A - Compiler flags ok
N/A - Debuginfo complete


Issues:
- Missing Requires: gnome-python2-gconf (this will also pull in all other deps such as pygtk2)
- require NetworkManager (or even NetworkManager-gnome)
- Description: change "network manager" to "NetworkManager"
- I'm not really happy with %{_bindir}/trafficd. How about renaming it to gtrafficd? Just an idea and only if it doesn't cause too much trouble.

Comment 2 Thomas Spura 2009-11-05 08:11:09 UTC
* Thu Nov 05 2009 Thomas Spura <tomspur> - 1.01-3
- Missing R: NetworkManager, BR: gnome-python2-gconf
- only just %%{buildroot}
- change trafficd into gtrafficd

I'm not really happy with renaming to gtrafficd, because it's a decision from upstram. If other users from other distros have problems and want to search for a solution, they all might get confused. But as this is a really small and possibly unknown programm, this would be, ok.

So I renamed it in the spec, your choice, what to use. ;)


Spec URL: http://tomspur.fedorapeople.org/review/gtraffic.spec
SRPM URL: http://tomspur.fedorapeople.org/review/gtraffic-1.01-3.fc11.src.rpm

Comment 3 Christoph Wickert 2009-11-05 12:16:54 UTC
Hmm, I don't like the way you rename trafficd. Why cp and rm, why not simply mv? I don't even like touching things in %{buildroot} after install, IMO this should be patched in the Makefile/in setup.py. After all, it's us decision if you want to rename the program, I wouldn't call it a blocker. ATM there are no collisions with trafficd, but in long term at least theoretically there could be some.

I suggest to leave it trafficd for now and ask upstream if renaming is ok for him. If you need something to support your point of view, tell him about
https://fedoraproject.org/wiki/Packaging_tricks#Use_of_common_namespace

Items to check:
Ok - Requires: are correct now
OK - Spelling of NetworkManager is correct

One last thing: Changelog is wrong: gnome-python2-gconf is R instead of BR. I suggest you change that line to "Require NetworkManager and gnome-python2-gconf", this is what everybody will understand.

Anyway, the package is APPROVED

Comment 4 Thomas Spura 2009-11-06 08:45:05 UTC
Thanks for reviewing this.

I've corrected the changelog and asked upstream about renaming, but will wait for an answer till the first checkin...


New Package CVS Request
=======================
Package Name: gtraffic
Short Description: Simple traffic usage counter for mobile broadband connections
Owners: cwickert, tomspur
Branches: F-11 F-12 EL-5
InitialCC:

Comment 5 Thomas Spura 2009-11-06 14:35:51 UTC
- Upstream agreed to the rename.
- I used cp and rm and not mv, because I thought mv would not preserve the timestamps, but now using mv.
- renaming is now in %prep and not in %install anymore

Spec URL: http://tomspur.fedorapeople.org/review/gtraffic.spec
SRPM URL: http://tomspur.fedorapeople.org/review/gtraffic-1.01-4.fc11.src.rpm

Comment 6 Christoph Wickert 2009-11-06 19:13:15 UTC
Much better, thanks!

Comment 7 Kevin Fenzi 2009-11-06 20:23:13 UTC
cvs done.

Comment 8 Fedora Update System 2009-11-07 00:16:34 UTC
gtraffic-1.01-4.fc12 has been submitted as an update for Fedora 12.
http://admin.fedoraproject.org/updates/gtraffic-1.01-4.fc12

Comment 9 Fedora Update System 2009-11-07 00:17:16 UTC
gtraffic-1.01-4.fc11 has been submitted as an update for Fedora 11.
http://admin.fedoraproject.org/updates/gtraffic-1.01-4.fc11

Comment 10 Fedora Update System 2009-11-10 17:44:30 UTC
gtraffic-1.01-4.fc11 has been pushed to the Fedora 11 testing repository.  If problems still persist, please make note of it in this bug report.
 If you want to test the update, you can install it with 
 su -c 'yum --enablerepo=updates-testing update gtraffic'.  You can provide feedback for this update here: http://admin.fedoraproject.org/updates/F11/FEDORA-2009-11177

Comment 11 Fedora Update System 2009-11-10 17:49:21 UTC
gtraffic-1.01-4.fc12 has been pushed to the Fedora 12 stable repository.  If problems still persist, please make note of it in this bug report.

Comment 12 Fedora Update System 2009-11-20 05:13:36 UTC
gtraffic-1.01-4.fc11 has been pushed to the Fedora 11 stable repository.  If problems still persist, please make note of it in this bug report.


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