Bug 637923
Summary: | Review Request: firmware-extract - A firmware-tools plugin to add firmware extraction from vendor binaries | ||
---|---|---|---|
Product: | [Fedora] Fedora | Reporter: | Matt Domsch <matt_domsch> |
Component: | Package Review | Assignee: | Praveen K Paladugu <praveen_paladugu> |
Status: | CLOSED RAWHIDE | QA Contact: | Fedora Extras Quality Assurance <extras-qa> |
Severity: | medium | Docs Contact: | |
Priority: | medium | ||
Version: | rawhide | CC: | fedora-package-review, linux-bugs, michael_e_brown, notting, praveen_paladugu |
Target Milestone: | --- | Flags: | praveen_paladugu:
fedora-review+
kevin: fedora-cvs+ |
Target Release: | --- | ||
Hardware: | All | ||
OS: | Linux | ||
Whiteboard: | |||
Fixed In Version: | Doc Type: | Bug Fix | |
Doc Text: | Story Points: | --- | |
Clone Of: | Environment: | ||
Last Closed: | 2011-03-24 14:11:46 UTC | Type: | --- |
Regression: | --- | Mount Type: | --- |
Documentation: | --- | CRM: | |
Verified Versions: | Category: | --- | |
oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |
Cloudforms Team: | --- | Target Upstream Version: | |
Embargoed: |
Description
Matt Domsch
2010-09-27 18:43:05 UTC
Charles/Sri - maybe something your team can review? (Fedora new package review process - Michael Brown is the upstream owner.) Ah, I see Praveen grabbed it. Thanks! 1) Please remove the suse related macros from the spec file. 2) Please update the pythong_sitelib definition with conditionals as shown at: http://fedoraproject.org/wiki/Packaging:Python#Macros 3) I don't see a need for the following lines in the spec file: touch configure find . -type f -newer configure -print0 | xargs -r0 touch 4) Please update the spec file to use either $RPM_BUILD_ROOT/%{buildroot} instead of both. 5) Please add a changelog comment to be in consistent with the current versioning. Nothing stands out in the rpmlint errors :). Praveen 1) there are two compat statements for suse compatibility. (6 total LOC, plus associated comments) This is not excessive and doesnt violate packaging standards, as far as I know. 2) The %{!? construct will only define the macro if it is not already defined. Thus, adding an %if only adds extra lines of code without technically changing the behaviour at all. I've changed the comment. 3) I've seen problems on when the system I create the tarball on has a time that is faster than the buildsystem time. These two lines fix the build for this corner case. I've removed them for now, as the build system which I used that had this problem seems to be fixed now. 4) Fixed. 5) fixed I've pushed the spec file updates to master git, but I have not created a new release since this was only spec updates without code changes. http://domsch.com/linux/fedora/firmware-extract/ now has the updated spec and SRPM. 1))) It certainly doesn't violate any rules. Just wanted to clean up the spec file of dead code/commands. Let it be for now. 2))) Got it. Thanks 4))) I still see both $RPM_BUILD_ROOT and %{buildroot} being used in the updated spec file. Praveen 4) indeed. oops. Fixed, good spot! Fixes pushed to git master. The latest update in git looks good. Please go ahead and request Fedora GIT access. Praveen Forgot to mention, please upload the final version to a publicly accessible location for future reference. Praveen New Package SCM Request ======================= Package Name: firmware-extract Short Description: A firmware-tools plugin to add firmware extraction from vendor binaries Owners: mdomsch mebrown praveenp Branches: f12 f13 f14 el4 el5 el6 InitialCC: http://domsch.com/linux/fedora/firmware-extract/ has the final SRPM and spec. Git done (by process-git-requests). |