Bug 2009498

Summary: Review Request: gojq - Pure Go implementation of jq
Product: [Fedora] Fedora Reporter: Mikel Olasagasti Uranga <mikel>
Component: Package ReviewAssignee: Maxwell G <maxwell>
Status: CLOSED ERRATA QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: medium Docs Contact:
Priority: medium    
Version: rawhideCC: maxwell, package-review, sanne.raymaekers
Target Milestone: ---Flags: maxwell: 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: 2021-12-07 22:52:56 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:
Bug Depends On: 2009494, 2009496    
Bug Blocks: 1803302, 2010367    

Description Mikel Olasagasti Uranga 2021-09-30 19:26:06 UTC
Spec URL: https://mikel.olasagasti.info/tmp/fedora/golang-github-itchyny-gojq.spec
SRPM URL: https://mikel.olasagasti.info/tmp/fedora/golang-github-itchyny-gojq-0.12.5-1.fc34.src.rpm
Description: Pure Go implementation of jq
Fedora Account System Username: mikelo2

Comment 1 Maxwell G 2021-12-05 04:29:52 UTC
`0.12.6` was released a couple days ago. You should update to that. I don't think this is required, but I would recommend getting rid of `cmd/*` and other globs and explicitly refer to gojq. Also, what do you think about renaming the package that provides the `gojq` binary to `gojq`?

Comment 2 Mikel Olasagasti Uranga 2021-12-05 07:57:41 UTC
0.12.6 removes deps on golang-github-itchyny-flags, so yes, I'll update it asap.

About naming gojq, it makes sense. I'll adapt the spec file.

Comment 3 Sanne Raymaekers 2021-12-05 11:57:53 UTC
*** Bug 2028445 has been marked as a duplicate of this bug. ***

Comment 5 Maxwell G 2021-12-07 20:13:03 UTC
I am not going to go through the whole fedora-review template, as this package uses go2rpm. I reviewed based on this copr build: https://download.copr.fedorainfracloud.org/results/mikelo2/github-cli/fedora-rawhide-x86_64/03005882-gojq/

- The specfile is sane.
- License is correct
- Builds successfully in mock
- No rpmlint errors
- %check section passes

I have one other comment:

> Name:           gojq

I would change this to `Name: %{goname}` (but with the proper indentation) and then add `%global goname gojq` right below `%gometa` (with an empty line in between). You should do this for the rest of the packages where you overrode the `Name`, instead of completely removing `%goname`.


Also, there are still two globs left in the specfile:

> install -m 0755 -vp %{gobuilddir}/cmd/* %{buildroot}%{_bindir}/

> %{_bindir}/*

I like to avoid globs in my specfiles, when possible, but I guess it's up to you whether to keep them or not. This is how go2rpm generates specfiles, and it's not against the Go Packaging Guidelines.

When I aproove this package and you import it, don't forget to add `Fixes rhbz#2009498` to the rpm changelog and/or manually add this bug to the Bodhi update so it gets marked as CLOSED. I also added your package to Antiya/release-monitoring.org. I think you should give go-sig admin privileges when the package is imported, as well.

Thanks,
Maxwell

Comment 6 Mikel Olasagasti Uranga 2021-12-07 20:52:13 UTC
Spec URL: https://mikel.olasagasti.info/tmp/fedora/gojq.spec
SRPM URL: https://mikel.olasagasti.info/tmp/fedora/gojq-0.12.6-1.fc35.src.rpm

- Added `%global goname gojq` to use it Name
- Remove rest of globs from %install and %files

Comment 7 Maxwell G 2021-12-07 21:56:14 UTC
Package approved! Thanks for putting up with all my nitpicks 😄.

Comment 8 Mikel Olasagasti Uranga 2021-12-07 22:15:54 UTC
Thank you for your patience!

Comment 9 Gwyn Ciesla 2021-12-07 22:20:12 UTC
(fedscm-admin):  The Pagure repository was created at https://src.fedoraproject.org/rpms/gojq

Comment 10 Fedora Update System 2021-12-07 22:51:59 UTC
FEDORA-2021-dd6a32be55 has been submitted as an update to Fedora 36. https://bodhi.fedoraproject.org/updates/FEDORA-2021-dd6a32be55

Comment 11 Fedora Update System 2021-12-07 22:52:56 UTC
FEDORA-2021-dd6a32be55 has been pushed to the Fedora 36 stable repository.
If problem still persists, please make note of it in this bug report.