Bug 783655

Summary: Review Request: aimage - Advanced Disk Imager
Product: [Fedora] Fedora Reporter: Nicolas Chauvet (kwizart) <kwizart>
Component: Package ReviewAssignee: Volker Fröhlich <volker27>
Status: CLOSED WONTFIX QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: medium Docs Contact:
Priority: medium    
Version: rawhideCC: notting, package-review, volker27
Target Milestone: ---Flags: volker27: fedora-review?
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2012-07-22 17:45:35 UTC Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:

Description Nicolas Chauvet (kwizart) 2012-01-21 14:47:08 UTC
Spec URL:
http://kwizart.fedorapeople.org/review/aimage.spec
SRPM URL:
http://kwizart.fedorapeople.org/review/aimage-3.2.5-1.fc17.src.rpm
Description: Advanced Disk Imager

Upstream has resurrected this package to make it compliant with newer afflib.

Comment 1 Volker Fröhlich 2012-01-21 23:51:28 UTC
You can remove defattr.

It seems to me, there is no afflib in EL. If I'm right and you don't want to introduce it there, you can drop everything specific for EPEL 5 packaging:

- Buildroot definition
- rm in install section
- clean section

More important things:

- Add COPYING to %doc
- Resolve the format warnings in the build
- "WARNING: unrecognized options: --enable-opt"
- Does aimage make use of ncurses and readline? It seeks for them and also tries to link.

Comment 2 Nicolas Chauvet (kwizart) 2012-01-23 22:57:16 UTC
Spec URL:
http://kwizart.fedorapeople.org/review/aimage.spec
SRPM URL:
http://kwizart.fedorapeople.org/review/aimage-3.2.5-2.fc17.src.rpm
Description: Advanced Disk Imager


Changelog:
- Bump afflib requirement
- Add COPYING
- Remove uneeded section
- Note on why readline is incompatible license wise
- Add BR ncurses-devel

Note that this package target f17.
Thx for the reviews.

Comment 3 Volker Fröhlich 2012-01-28 16:03:20 UTC
What is your intention with requireing a specific version of afflib-devel? I note you're targeting F17, but if the afflib version changes there, you have to update your spec file. If there is a good reason, please comment on the spec file.

I'd personally like to see a better description text.

Resolving the format warnings in the build should be fairly easy and not bare any risk. Please do it, if possible and submit it to the author/s.

Besides that everything looks fine, I'll take the review.

Comment 4 Volker Fröhlich 2012-01-28 16:14:15 UTC
Can you gather any kind of documentation?

Comment 5 Nicolas Chauvet (kwizart) 2012-03-14 18:26:54 UTC
Hi,
I think the spec file is obvious. aimage requires at least a specific version of afflib, period. 
That's exactly what BuildRequires afflib >= 3.6.15 means.
aimage/afflib are both from afflib.org, so when one is updated another comes along.

A previous aimage used to be in earlier fedora. I have orphaned it because it was orphaned upstream over another tool. But as it has been resurrected, that's need a new review.

Which format-warning are you talking about ?
There is no documentation
The description was picked from the website:
http://afflib.org/software/aimage-the-advanced-disk-imager

Thx for the review

Comment 6 Nicolas Chauvet (kwizart) 2012-07-22 17:45:35 UTC
aimage remains in in-between state, but since the FTBFS is fixed that good for me to let it go without a maintainer.