Note: This is a public test instance of Red Hat Bugzilla. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback at bugzilla.redhat.com.
Bug 1375986 - Review Request: golang-github-klauspost-cpuid - CPU feature identification for Go
Summary: Review Request: golang-github-klauspost-cpuid - CPU feature identification fo...
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Fabio Valentini
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
: 1437463 (view as bug list)
Depends On:
Blocks: 1376389 1376749 1437471
TreeView+ depends on / blocked
 
Reported: 2016-09-14 12:10 UTC by Matthias Runge
Modified: 2017-05-03 05:58 UTC (History)
2 users (show)

Fixed In Version:
Doc Type: If docs needed, set a value
Doc Text:
Clone Of:
Environment:
Last Closed: 2017-05-02 15:56:25 UTC
Type: ---
Embargoed:
decathorpe: fedora-review+


Attachments (Terms of Use)

Description Matthias Runge 2016-09-14 12:10:00 UTC
Spec URL: https://mrunge.fedorapeople.org/reviews/golang-github-klauspost-cpuid/golang-github-klauspost-cpuid.spec

SRPM URL: https://mrunge.fedorapeople.org/reviews/golang-github-klauspost-cpuid/golang-github-klauspost-cpuid-0-0.1.git09cded8.fc24.src.rpm

Description: CPU feature identification for Go

Fedora Account System Username: mrunge

Koji: http://koji.fedoraproject.org/koji/taskinfo?taskID=15630358

$ rpmlint golang-github-klauspost-cpuid-0-0.1.git09cded8.fc24.src.rpm golang-github-klauspost-cpuid-devel-0-0.1.git09cded8.fc24.noarch.rpm golang-github-klauspost-cpuid-unit-test-devel-0-0.1.git09cded8.fc24.x86_64.rpm
3 packages and 0 specfiles checked; 0 errors, 0 warnings.

Comment 2 Fabio Valentini 2017-03-30 14:20:17 UTC
*** Bug 1437463 has been marked as a duplicate of this bug. ***

Comment 3 Fabio Valentini 2017-03-30 14:23:29 UTC
Taking this review.

Initial recommendation: Please regenerate the .spec file with "gofed github2spec", it seems there's some legacy/outdated cruft in there. Also, if you package version 1.0, make sure to reference the correct %commit in the .spec file, too.

Comment 5 Fabio Valentini 2017-03-31 07:32:22 UTC
It looks like you made a copy-paste error, because this link is now pointing at the old version again.

Comment 6 Matthias Runge 2017-03-31 09:00:40 UTC
The revision is the same created from the spec file. But apparently I had to modify the spec to fill in the summary and license.

The commit fits together with the last commit on https://github.com/klauspost/cpuid

I updated the linked files.

Comment 7 Fabio Valentini 2017-03-31 11:41:58 UTC
It works now, thanks. Suggestions for improvements:

1) Since you are packaging version 1.0, you should set
> Version: 1.0
> Release: 1%{?dist}
and correct it the changelog entry too (1.0-1).

2) You can drop the empty %if-endif blocks at lines 94, 86 and 68.

Comment 8 Fabio Valentini 2017-03-31 11:42:20 UTC
Additionally, a koji scratch build for rawhide would be nice.

Comment 9 Fabio Valentini 2017-04-14 09:56:21 UTC
Pinging submitter.

I would like to finish up this review, since some of my pending packages depend on it.

Comment 11 Fabio Valentini 2017-04-18 08:12:52 UTC
There's only one major issue left as far as I can see: The Release: tag is "1", when it should be "1%{?dist}". No other changes are necessary for this.

A cosmetic issue is that the conditional block between lines 141 and 147 is redundant, since the expression is always the same, so you could replace this:

> %if ! 0%{?with_bundled}
> export GOPATH=%{buildroot}/%{gopath}:%{gopath}
> %else
> # No dependency directories so far
> 
> export GOPATH=%{buildroot}/%{gopath}:%{gopath}
> %endif

with just this:

> export GOPATH=%{buildroot}/%{gopath}:%{gopath}

Otherwise, the package looks good. If you fix the Release tag, I will approve the package. I will leave it up to you if you want to clean up the redundant conditional.

Comment 12 Matthias Runge 2017-04-18 09:55:22 UTC
(In reply to Fabio Valentini from comment #11)

> > %if ! 0%{?with_bundled}
> > export GOPATH=%{buildroot}/%{gopath}:%{gopath}
> > %else
> > # No dependency directories so far
> > 
> > export GOPATH=%{buildroot}/%{gopath}:%{gopath}
> > %endif
> 
> with just this:
> 
> > export GOPATH=%{buildroot}/%{gopath}:%{gopath}

Good catch!

I fixed both.

Updated
SPEC: http://www.matthias-runge.de/fedora/golang-github-klauspost-cpuid.spec
SRPM: http://www.matthias-runge.de/fedora/golang-github-klauspost-cpuid-1.0-1.fc27.src.rpm

Comment 13 Fabio Valentini 2017-04-18 10:02:46 UTC
Looks good now. Package approved.

Comment 14 Fabio Valentini 2017-04-18 16:51:24 UTC
By the way, please select at least f24, f25, f26 and master when you submit the package request. I need this package on those 4 releases :)

Comment 15 Gwyn Ciesla 2017-04-20 11:41:53 UTC
Package request has been approved: https://admin.fedoraproject.org/pkgdb/package/rpms/golang-github-klauspost-cpuid

Comment 16 Fedora Update System 2017-04-21 10:50:11 UTC
golang-github-klauspost-cpuid-1.0-1.fc26 has been submitted as an update to Fedora 26. https://bodhi.fedoraproject.org/updates/FEDORA-2017-8d3f8fe472

Comment 17 Fedora Update System 2017-04-21 11:41:00 UTC
golang-github-klauspost-cpuid-1.0-1.fc24 has been submitted as an update to Fedora 24. https://bodhi.fedoraproject.org/updates/FEDORA-2017-a09b41108c

Comment 18 Fedora Update System 2017-04-21 11:41:27 UTC
golang-github-klauspost-cpuid-1.0-1.fc25 has been submitted as an update to Fedora 25. https://bodhi.fedoraproject.org/updates/FEDORA-2017-9e25c40a33

Comment 19 Fedora Update System 2017-04-23 17:21:08 UTC
golang-github-klauspost-cpuid-1.0-1.fc25 has been pushed to the Fedora 25 testing repository. If problems still persist, please make note of it in this bug report.
See https://fedoraproject.org/wiki/QA:Updates_Testing for
instructions on how to install test updates.
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2017-9e25c40a33

Comment 20 Fedora Update System 2017-04-23 20:24:03 UTC
golang-github-klauspost-cpuid-1.0-1.fc26 has been pushed to the Fedora 26 testing repository. If problems still persist, please make note of it in this bug report.
See https://fedoraproject.org/wiki/QA:Updates_Testing for
instructions on how to install test updates.
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2017-8d3f8fe472

Comment 21 Fedora Update System 2017-05-02 15:56:25 UTC
golang-github-klauspost-cpuid-1.0-1.fc26 has been pushed to the Fedora 26 stable repository. If problems still persist, please make note of it in this bug report.

Comment 22 Fedora Update System 2017-05-02 23:14:30 UTC
golang-github-klauspost-cpuid-1.0-1.fc25 has been pushed to the Fedora 25 stable repository. If problems still persist, please make note of it in this bug report.

Comment 23 Fedora Update System 2017-05-03 05:58:20 UTC
golang-github-klauspost-cpuid-1.0-1.fc24 has been pushed to the Fedora 24 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.