Bug 444264 - Review Request: usb_modeswitch - brings umts / 4g cards into operational mode
Review Request: usb_modeswitch - brings umts / 4g cards into operational mode
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Christoph Wickert
Fedora Extras Quality Assurance
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2008-04-26 04:00 EDT by mail@romal.de
Modified: 2008-07-02 23:16 EDT (History)
3 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2008-07-02 00:49:27 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
cwickert: fedora‑review+
kevin: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description mail@romal.de 2008-04-26 04:00:46 EDT
Spec URL: http://www.romal.de/files/usb_modeswitch.spec
SRPM URL: http://www.romal.de/files/usb_modeswitch-0.9.4beta2-1.fc9.src.rpm
Description: USB Modeswitch brings up your datacard into operational mode. When plugged in they identify themself as cdrom and present some non-Linux compatible installation files. This tool deactivates this cdrom-devices and enables the real communication device. It supports most devices built and sold by Huawei, T-Mobile, Vodafone, Option, ZTE, Novatel.
Comment 1 mail@romal.de 2008-04-26 04:03:52 EDT
This is my first rpm, so it may be horrible :-O
Comment 2 Christoph Wickert 2008-06-06 04:04:55 EDT
Version and Release are wrong:
Version should be 0.9.4 and then Release will become 0.1.beta2%{?dist}
A new release of the same package will become 0.2.beta2%{?dist} and when 0.9.4
becomes stable you switch to 1%{?dist}. See
http://fedoraproject.org/wiki/Packaging/NamingGuidelines#Pre-Release_packages

Description needs line breaks after 79 characters.

You can remove "Requires: libusb", rpm will pick this up automagically:
  $ rpm -qp --requires usb_modeswitch-0.9.4beta2-1.fc9.i386.rpm 
  libc.so.6  
  libc.so.6(GLIBC_2.0)  
  libc.so.6(GLIBC_2.1)  
  libusb           <--this is from the Requires:
  libusb-0.1.so.4  <--from rpm
  rpmlib(CompressedFileNames) <= 3.0.4-1
  rpmlib(PayloadFilesHavePrefix) <= 4.0-1
  rtld(GNU_HASH)

Don't use compile.sh because it doesn't honor compiler flags, see
http://fedoraproject.org/wiki/Packaging/Guidelines#Compiler_flags
Use
  gcc $RPM_OPT_FLAGS -l usb -o usb_modeswitch usb_modeswitch.c
instead. rpmbuild will take care of and stripping the binaries, so this should
not be done in compile.sh ether.

The rest looks good to me. Please fix the errors I mentioned above in a new
0.2.beta2 package and then I'll do the final review.
Comment 3 mail@romal.de 2008-06-22 14:31:17 EDT
Spec URL: http://www.romal.de/files/usb_modeswitch.spec
SRPM URL: http://www.romal.de/files/usb_modeswitch-0.9.4-1.fc9.src.rpm

Done.

I fixed the spec-file and synced the new upstream release.
Comment 4 Christoph Wickert 2008-06-22 16:17:58 EDT
Review for
847c6b0838ceda2c3c794748bd15446e  usb_modeswitch-0.9.4-1.fc9.src.rpm

FIX - MUST: rpmlint:

  rpmlint /var/lib/mock/fedora-rawhide-i386/result/usb_modeswitch-*
  usb_modeswitch.i386: W: non-conffile-in-etc /etc/usb_modeswitch.conf

The file needs to be marked as config file:
  %config(noreplace) %{_sysconfdir}/usb_modeswitch.conf

 see http://fedoraproject.org/wiki/Packaging/Guidelines#Configuration_files


  usb_modeswitch.i386: W: incoherent-version-in-changelog 0.9.4 0.9.4-1.fc10

The version needs to be specified in the format %{version}-%{release} without
disttag. This is important because changelog gets parsed automatically. And
_please_ preserve the whole changelog and make an entry for every package
release, even during review so we can track the changes. A correct changelog
would look like this:

  %changelog
  * Sat Jun 22 2008 Robert M. Albrecht <romal@gmx.de> 0.9.4-1
  - Update to 0.9.4
  - Honor RPM_OPT_FLAGS
  
  * Sat May 26 2008 Robert M. Albrecht <romal@gmx.de> 0.9.4-0.1.beta2
  - First package Release


  usb_modeswitch.src: W: mixed-use-of-spaces-and-tabs (spaces: line 1, tab: line 1)

I know that this is pedantic, but so is rpmlint. Ether use one, tabs OR spaces.
I wouldn't call this a blocker because as the specfile is legible, so fix this
if you like. I recommend to only use spaces for formatting because tabs are IMO
harder to read in cvs diffs.

OK - MUST: The package is named according to the  Package Naming Guidelines
OK - MUST: The spec file name matches the base package %{name}
OK - MUST: The package meets the  Packaging Guidelines
OK - MUST: The package is licensed with a Fedora approved license (GPLv2+)
OK - MUST: The License field in the package spec file matches the actual license
OK - MUST: The source package includes the text of the license and it is
correctly included in %doc
OK - MUST: The spec file is written in American English
OK - MUST: The spec file is legible
OK - MUST: The sources used to build the package match the upstream source by
md5 3c03c3ae51a10d599c33c119693186c9
OK - MUST: The package successfully compiles and builds into binary rpms on i386
OK - MUST: No known ExcludeArch
OK - MUST: All build dependencies are listed in BuildRequires
OK - MUST: The package is not designed to be relocatable
OK - MUST: The package owns all directories that it creates (it creates none)
OK - MUST: The package does not contain any duplicate files in the %files listing
OK - MUST: Permissions on files are set properly
OK - MUST: The package has a %clean section, which contains rm -rf %{buildroot}
OK - MUST: The package consistently uses macros, as described in the macros
section of Packaging Guidelines
OK - MUST: The package contains code, no content
OK - MUST: No large documentation files for a separate %{name}-doc package
OK - MUST: The files included as %doc do not affect the runtime of the application
OK - MUST: The package 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 in the package are valid UTF-8
OK - SHOULD: The package builds in mock

FIX - SHOULD: The description in the package spec file contains German
translations. Please also add a German Summary then:

  Summary(de):	USB Modeswitch aktiviert UMTS-Karten

And please use Umlauts.

OK - SHOULD: The package seems to function as described (cannot really test in
absence of an 4G Card)
OK - SHOULD: The package is the latest version


Please fix all issues I marked and then I will approve this package. NEEDSWORK.
Comment 6 Christoph Wickert 2008-06-22 17:50:05 EDT
rpmlint is happy and so am I. This package is APPROVED.

1. Please follow the instructions in
http://fedoraproject.org/wiki/PackageMaintainers/CVSAdminProcedure

2. You need to get a Fedora Account
http://fedoraproject.org/wiki/PackageMaintainers/Join#Get_a_Fedora_Account
I guess you already have one so you only need to apply for the "cvsextras" group.

3. You might also want to read
http://fedoraproject.org/wiki/PackageMaintainers/HowToGetSponsored
(Don't worry, I will sponsor you)
Comment 7 mail@romal.de 2008-06-27 05:38:05 EDT
New Package CVS Request
=======================
Package Name: usb_modeswitch
Short Description: Gets 4G cards in operational mode
Owners: romal
Branches: F-9
InitialCC: romal
Cvsextras Commits: yes
Comment 8 Kevin Fenzi 2008-06-29 00:20:27 EDT
I don't see 'romal' in the cvsextras group. 

Christoph: Did you sponsor him? Is the account name correct?
Comment 9 Christoph Wickert 2008-06-29 06:10:57 EDT
romal is correct and he now is sponsored.
Comment 10 Kevin Fenzi 2008-06-30 11:58:11 EDT
cvs done.
Comment 11 Fedora Update System 2008-07-02 00:48:05 EDT
usb_modeswitch-0.9.4-2.fc9 has been submitted as an update for Fedora 9
Comment 12 Fedora Update System 2008-07-02 23:16:07 EDT
usb_modeswitch-0.9.4-2.fc9 has been pushed to the Fedora 9 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.