Bug 2101769

Summary: Review Request: pf-bb-config - PF BBDEV (baseband device) Configuration Application
Product: [Fedora] Fedora Reporter: Timothy Redaelli <tredaelli>
Component: Package ReviewAssignee: Neal Gompa <ngompa13>
Status: CLOSED ERRATA QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: medium Docs Contact:
Priority: medium    
Version: rawhideCC: maxime.coquelin, ngompa13, package-review, trix
Target Milestone: ---Flags: ngompa13: fedora-review+
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: Doc Type: If docs needed, set a value
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2022-10-07 10:03:22 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 Timothy Redaelli 2022-06-28 10:44:57 UTC
Spec URL: https://tredaell.fedorapeople.org/pf-bb-config/pf-bb-config.spec
SRPM URL: https://tredaell.fedorapeople.org/pf-bb-config/pf-bb-config-22.03-1.fc36.src.rpm
Description:
The PF BBDEV (baseband device) Configuration Application "pf_bb_config"
provides a means to configure the baseband device at the host-level.
The program accesses the configuration space and sets the various parameters
through memory-mapped IO read/writes.
Fedora Account System Username: tredaell

Comment 1 Neal Gompa 2022-10-03 15:49:14 UTC
Taking this review.

Comment 2 Neal Gompa 2022-10-03 15:51:53 UTC
> Source0:        https://github.com/intel/pf-bb-config/archive/refs/tags/v%{version}.tar.gz#/pf-bb-config-%{version}.tar.gz

This should be simplified to the following: "%{url}/archive/v%{version}/pf-bb-config-%{version}.tar.gz"

You're also missing "BuildRequires: make"

> %{_bindir}/*
> %{_datadir}/*

This is not specific enough.

Cf. https://docs.fedoraproject.org/en-US/packaging-guidelines/#_explicit_lists

Comment 3 Timothy Redaelli 2022-10-03 17:34:23 UTC
(In reply to Neal Gompa from comment #2)
> > Source0:        https://github.com/intel/pf-bb-config/archive/refs/tags/v%{version}.tar.gz#/pf-bb-config-%{version}.tar.gz
> 
> This should be simplified to the following:
> "%{url}/archive/v%{version}/pf-bb-config-%{version}.tar.gz"
> 
> You're also missing "BuildRequires: make"
> 
> > %{_bindir}/*
> > %{_datadir}/*
> 
> This is not specific enough.
> 
> Cf.
> https://docs.fedoraproject.org/en-US/packaging-guidelines/#_explicit_lists

I updated the spec file and the srpm with the required modifications and, in the meanwhile, I also rebased it with the last upstream version.
I hope it's ok now

Spec URL: https://tredaell.fedorapeople.org/pf-bb-config/pf-bb-config.spec
SRPM URL: https://tredaell.fedorapeople.org/pf-bb-config/pf-bb-config-22.07-1.fc38.src.rpm

Thank you for your review

Comment 4 Neal Gompa 2022-10-05 13:50:56 UTC
> %{_datadir}/pf-bb-config/acc100/acc100_config.cfg
> %{_datadir}/pf-bb-config/acc100/acc100_config_1vf_4g5g.cfg
> %{_datadir}/pf-bb-config/acc100/acc100_config_1vf_5g.cfg
> %{_datadir}/pf-bb-config/acc100/acc100_config_2vf_4g5g.cfg
> %{_datadir}/pf-bb-config/acc100/acc100_config_4vf_4g5g.cfg
> %{_datadir}/pf-bb-config/acc100/acc100_config_pf.cfg
> %{_datadir}/pf-bb-config/acc100/acc100_config_pf_4g5g.cfg
> %{_datadir}/pf-bb-config/acc100/acc100_config_vf.cfg
> %{_datadir}/pf-bb-config/acc100/acc100_config_vf_4g.cfg
> %{_datadir}/pf-bb-config/acc100/acc100_config_vf_5g.cfg

You can simplify this to just:

%{_datadir}/pf-bb-config/

Comment 5 Timothy Redaelli 2022-10-05 15:02:17 UTC
Done, thank you

Comment 6 Neal Gompa 2022-10-05 15:17:34 UTC
Package review notes:

* Package follows packaging guidelines
* Package licensing is correct, license files installed correctly (though you should swap "ASL 2.0" for "Apache-2.0" in the license tag)
* Package builds and installs correctly
* No serious rpmlint issues

PACKAGE APPROVED.

Comment 7 Gwyn Ciesla 2022-10-05 19:06:31 UTC
(fedscm-admin):  The Pagure repository was created at https://src.fedoraproject.org/rpms/pf-bb-config

Comment 8 Fedora Update System 2022-10-07 10:01:35 UTC
FEDORA-2022-13a7f2feb7 has been submitted as an update to Fedora 38. https://bodhi.fedoraproject.org/updates/FEDORA-2022-13a7f2feb7

Comment 9 Fedora Update System 2022-10-07 10:03:22 UTC
FEDORA-2022-13a7f2feb7 has been pushed to the Fedora 38 stable repository.
If problem still persists, please make note of it in this bug report.