[Bug 494171] Review Request: hostapd - WLAN Accesspoint daemon

[bugzilla at redhat.com]

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=494171





--- Comment #6 from Adel Gadllah <adel.gadllah at gmail.com>  2009-04-08 15:30:46 EDT ---
(In reply to comment #3)
> Note that it's a practice review, I need to do some in order to get a
> sponsoring.

OK, thanks for looking at the package.

> * Naming: OK
> * Version and release: OK
> * Legal: OK
> * No inclusion of pre-built binaries or libraries: OK
> * Spec Legibility: OK
> * Writing a package from scratch: OK
> * Modifying an existing package: N/A
> * Architecture Support: koji has built the package correctly, so I assume OK
> * Filesystem layout: OK
> - %{_datadir}/man/man5/hostapd.* - did you consider using %{_mandir} ?

OK, will fix.

> * Use rpmlint: OK
> * Changelogs: OK
> * Tags: OK
> * BuildRoot tag: OK
> * Requires: OK
> * BuildRequires: OK
> * Summary and description: OK
> * Documentation: not-OK
> - Did you think about including hostapd/README?

Missed that one, thanks will add.

> - Did you think about building the documentation from hostapd/doc/ directory?
> I've made you a list of buildrequires for that: transfig, netpbm-progs,
> doxygen, graphviz, texlive-latex. I'm not sure if it's really necessary, since
> it contains information about API and friends and would make a little use for
> people outside the project.

This docs are mostly useless. 
There are only relevant if you want to hack on hostapd itself, in this case you
can build them or read the comments in the code. 

> * Compiler flags: OK
> * Debuginfo packages: OK
> * Devel packages: N/A
> * Requiring Base Package: N/A
> * Shared Libraries: N/A
> * Packaging Static Libraries: N/A
> * Duplication of system libraries: OK
> * Beware of Rpath: OK
> * Configuration files: OK
> * Initscripts: not-OK
> - The package would certainly benefit from initscript, since it's a system
> daemon.

Sure can add one.

> * Desktop files: N/A
> * Macros: mainly OK
> - See my comment for "Filesystem layout"
> - You are mixing %{optflags} with $RPM_BUILD_ROOT. In my opinion it's not
> really bad, 

That does not matter at all.

> * Handling Locale Files: N/A, hostapd is not localised.
> * Timestamps: not-OK
> - Add -p to your install commands.

OK

> * Parallel make: OK
> * Scriptlets: OK
> * Conditional dependencies: OK
> * Build packages with separate user accounts: not-OK ;)

???

> * Relocatable packages: OK
> * Code Vs Content: OK
> * File and Directory Ownership: OK
> * Users and Groups: N/A, hostapd requires root priviledges, doesn't it?

Yes it does, so what? ;) (has nothing to do on how it is packaged, it will
complain if you don't run it as root).

Moved the files to /usr/sbin/ instead of /usr/bin/ to make this more clear.

> * Web Applications: N/A
> * Conflicts: OK
> * No External Kernel Modules: OK
> * No Files or Directories under /srv: OK
> * Bundling of multiple projects: OK
> * All patches should have an upstream bug link or comment: not-OK
> - The Patch1 is Fedora-specific, but it doesn't say so. There is no description
> of it neither.

The patch is obivious (fixes the path in the makefile), but sure can add a
comment.


> * Application Specific Guidelines: N/A  

New spec & package:

http://193.200.113.196/apache2-default/rpm/hostapd.spec
SRPM URL:
http://193.200.113.196/apache2-default/rpm/hostapd-0.6.9-0.2.20090405gita0b2f99.fc10.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.