Bug 2169238 - Review Request: whatip - Info on your IP address
Summary: Review Request: whatip - Info on your IP address
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Neal Gompa
QA Contact: Fedora Extras Quality Assurance
URL: https://gitlab.gnome.org/GabMus/whatip
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2023-02-13 02:00 UTC by Yaakov Selkowitz
Modified: 2023-02-23 02:18 UTC (History)
2 users (show)

Fixed In Version:
Doc Type: If docs needed, set a value
Doc Text:
Clone Of:
Environment:
Last Closed: 2023-02-23 02:18:34 UTC
Type: ---
Embargoed:
ngompa13: fedora-review+


Attachments (Terms of Use)
The .spec file difference from Copr build 5520349 to 5522752 (843 bytes, patch)
2023-02-13 17:27 UTC, Jakub Kadlčík
no flags Details | Diff

Description Yaakov Selkowitz 2023-02-13 02:00:57 UTC
Spec URL: https://yselkowitz.fedorapeople.org/whatip.spec
SRPM URL: https://yselkowitz.fedorapeople.org/whatip-1.1-1.fc39.src.rpm
Description: What IP displays information on your local network, IP addresses and open ports.
Fedora Account System Username: yselkowitz

Comment 1 Jakub Kadlčík 2023-02-13 02:09:57 UTC
Copr build:
https://copr.fedorainfracloud.org/coprs/build/5520349
(succeeded)

Review template:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2169238-whatip/fedora-rawhide-x86_64/05520349-whatip/fedora-review/review.txt

Please take a look if any issues were found.

---
This comment was created by the fedora-review-service
https://github.com/FrostyX/fedora-review-service

If you want to trigger a new Copr build, add a comment containing new
Spec and SRPM URLs or [fedora-review-service-build] string.

Comment 2 Neal Gompa 2023-02-13 05:03:30 UTC
Spec review:

> desktop-file-validate %{buildroot}%{_datadir}/applications/%{app_id}.desktop
> appstream-util validate-relax --nonet %{buildroot}%{_metainfodir}/%{app_id}.appdata.xml

Nit: These should be in %check so that it's obvious they're part of package verification

> %{python3_sitelib}/%{name}

Please add a trailing slash so that it's obvious the RPM that this is a directory.

> %{_datadir}/icons/hicolor/*/*/%{app_id}*

This means we need "Requires: hicolor-icon-theme" in the package.

> %{_datadir}/%{name}

Please add a trailing slash so that it's obvious the RPM that this is a directory.

Comment 3 Yaakov Selkowitz 2023-02-13 17:16:21 UTC
(In reply to Neal Gompa from comment #2)
> Nit: These should be in %check so that it's obvious they're part of package verification

The guidelines state either %check or %install without preference.

> Please add a trailing slash so that it's obvious the RPM that this is a directory.

I'm not against this, but this seems to be a style choice rather than a guidelines question.

> This means we need "Requires: hicolor-icon-theme" in the package.

Added.

Comment 4 Yaakov Selkowitz 2023-02-13 17:16:33 UTC
Spec URL: https://yselkowitz.fedorapeople.org/whatip.spec
SRPM URL: https://yselkowitz.fedorapeople.org/whatip-1.1-1.fc39.src.rpm
Description: What IP displays information on your local network, IP addresses and open ports.
Fedora Account System Username: yselkowitz

Comment 5 Jakub Kadlčík 2023-02-13 17:27:10 UTC
Created attachment 1943870 [details]
The .spec file difference from Copr build 5520349 to 5522752

Comment 6 Jakub Kadlčík 2023-02-13 17:27:13 UTC
Copr build:
https://copr.fedorainfracloud.org/coprs/build/5522752
(succeeded)

Review template:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2169238-whatip/fedora-rawhide-x86_64/05522752-whatip/fedora-review/review.txt

Please take a look if any issues were found.

---
This comment was created by the fedora-review-service
https://github.com/FrostyX/fedora-review-service

If you want to trigger a new Copr build, add a comment containing new
Spec and SRPM URLs or [fedora-review-service-build] string.

Comment 7 Neal Gompa 2023-02-14 00:07:30 UTC
(In reply to Yaakov Selkowitz from comment #3)
> (In reply to Neal Gompa from comment #2)
> > Nit: These should be in %check so that it's obvious they're part of package verification
> 
> The guidelines state either %check or %install without preference.
> 
> > Please add a trailing slash so that it's obvious the RPM that this is a directory.
> 
> I'm not against this, but this seems to be a style choice rather than a
> guidelines question.
> 

We really should add this to the guidelines, but the reason for this is to prevent accidental directory->file transitions, which can cause RPM upgrade failures.

Comment 8 Yaakov Selkowitz 2023-02-14 00:44:17 UTC
(In reply to Neal Gompa from comment #7)
> We really should add this to the guidelines, but the reason for this is to
> prevent accidental directory->file transitions, which can cause RPM upgrade
> failures.

Generally speaking e.g. %{_datadir}/%{name} isn't about to become a file.  Any guidelines along these lines IMO should be limited in avoiding potential future collisions in specific known cases rather than dictating syntax/style.

In any case, I already added these in the previous update.

Comment 9 Neal Gompa 2023-02-14 10:24:08 UTC
Everything looks good, then.

PACKAGE APPROVED.

Comment 10 Fedora Admin user for bugzilla script actions 2023-02-14 12:13:43 UTC
The Pagure repository was created at https://src.fedoraproject.org/rpms/whatip

Comment 11 Fedora Update System 2023-02-14 16:18:26 UTC
FEDORA-2023-b55f632014 has been submitted as an update to Fedora 37. https://bodhi.fedoraproject.org/updates/FEDORA-2023-b55f632014

Comment 12 Fedora Update System 2023-02-15 01:34:48 UTC
FEDORA-2023-b55f632014 has been pushed to the Fedora 37 testing repository.
Soon you'll be able to install the update with the following command:
`sudo dnf install --enablerepo=updates-testing --refresh --advisory=FEDORA-2023-b55f632014 \*`
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2023-b55f632014

See also https://fedoraproject.org/wiki/QA:Updates_Testing for more information on how to test updates.

Comment 13 Fedora Update System 2023-02-23 02:18:34 UTC
FEDORA-2023-b55f632014 has been pushed to the Fedora 37 stable repository.
If problem still persists, 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.