Spec URL: https://decathorpe.fedorapeople.org/granite-7.spec SRPM URL: https://decathorpe.fedorapeople.org/granite-7-7.4.0-1.fc39.src.rpm Description: Granite is a companion library for GTK and GLib. Among other things, it provides complex widgets and convenience functions designed for use in apps built for elementary OS. Fedora Account System Username: decathorpe
This package built on koji: https://koji.fedoraproject.org/koji/taskinfo?taskID=109228205
Copr build: https://copr.fedorainfracloud.org/coprs/build/6666980 (succeeded) Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2250477-granite-7/fedora-rawhide-x86_64/06666980-granite-7/fedora-review/review.txt Found issues: - No gcc, gcc-c++ or clang found in BuildRequires Read more: https://docs.fedoraproject.org/en-US/packaging-guidelines/C_and_C++/ Please know that there can be false-positives. --- 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.
Would it make more sense to update granite[1] to 7.x and create a granite-6 compat package? That seems to be the usual approach to parallel-installable versions[2], although some GNOME/GTK packages have used other conventions. [1] https://src.fedoraproject.org/rpms/granite [2] https://docs.fedoraproject.org/en-US/packaging-guidelines/Naming/#multiple
I have thought about this, but it would be very awkward here. Essentially, granite-7 is a different project and not just version 7 of granite. granite V7 provides: - libgranite-7.so - granite-7 pkgconfig file - granite-7 headers and Vala bindings granite V6 provides; - libgranite.so - granite pkgconfig file - granite headers and Vala bindings I would like to stay closer to upstream / the actual library name. Packaging granite v7 as just "granite" and granite v6 as "granite6" would be an inversion of what's already getting installed.
s/already/actually/
That sounds reasonable. Thanks for writing down the rationale. I’m going to leave this review “open” for anyone, but it’s next on my list to pick up if nobody else gets to it first.
I don't love the name of this package, but I get it... That said... Package review notes: * Package (mostly) follows Fedora Packaging Guidelines * Package builds and installs * Package licensing is correctly handled * No serious issues from rpmlint The only issue is that you're missing a "BuildRequires: gcc", which you can add on import. PACKAGE APPROVED.
> I don't love the name of this package, but I get it... Me neither, but given how upstream has named the library, this is probably the best we can do without introducing more inconsistencies. That said... Thanks for the review! I'll continue with the rest of the Pantheon components.
I didn’t do a full review on this (or I would have assigned it to myself), but I did notice that the file lib/Services/AsyncMutex.vala is GPL-3.0-or-later, which is missing from the License expression. > The only issue is that you're missing a "BuildRequires: gcc", which you can add on import. It might be OK to rely on gcc being a transitive dependency of vala for a pure-Vala package like this, but there’s certainly no harm in adding it explicitly.
(In reply to Ben Beasley from comment #9) > I didn’t do a full review on this (or I would have assigned it to myself), > but I did notice that the file lib/Services/AsyncMutex.vala is > GPL-3.0-or-later, which is missing from the License expression. I'll double-check this before importing, thanks! > > The only issue is that you're missing a "BuildRequires: gcc", which you can add on import. > > It might be OK to rely on gcc being a transitive dependency of vala for a > pure-Vala package like this, but there’s certainly no harm in adding it > explicitly. Yeah, I don't think this is strictly necessary here, the "BR: vala" is already transitively pulling in a C compiler. But explicitly adding "BR: gcc" doesn't hurt, I guess.
The Pagure repository was created at https://src.fedoraproject.org/rpms/granite-7
Imported and built: https://bodhi.fedoraproject.org/updates/FEDORA-2023-ac475de0c0 I updated the license tag and added BR: gcc before importing the spec file.