Bug 2347045
| Summary: | Review Request: flatpak-app-config - Additional configuration files for Fedora Flatpak applications | ||
|---|---|---|---|
| Product: | [Fedora] Fedora | Reporter: | Yaakov Selkowitz <yselkowi> |
| Component: | Package Review | Assignee: | Neal Gompa <ngompa13> |
| Status: | CLOSED NEXTRELEASE | QA Contact: | Fedora Extras Quality Assurance <extras-qa> |
| Severity: | medium | Docs Contact: | |
| Priority: | medium | ||
| Version: | rawhide | CC: | ngompa13, package-review |
| Target Milestone: | --- | Keywords: | AutomationTriaged |
| Target Release: | --- | Flags: | ngompa13:
fedora-review+
|
| 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: | 2025-03-24 18:01:02 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
Yaakov Selkowitz
2025-02-21 22:05:36 UTC
Taking this review. Initial spec review: > %{-i:Source: %{appid}.appdata.xml}\ This is too close to conditional sources for my liking. We don't allow sources to be conditional in a spec file, they need to be unconditionally included in the SRPM. Please pull this out and use regular Source lines. > %define app_subpkg(n:i:) \ Wouldn't this make sense as an RPM macro that is used in the actual utility and library packages? > Supplements: (%{pkgname} and flatpak-runtime-config)\ Does this work in the flatpak build environment? This form of macro is used when packages need to create a large number of similar subpackages and avoid the boilerplate that usually entails. This package will be expanded as needed, hence I am building in flexibility now; not necessarily will every subpackage need to include an appdata file, even though this first batch does. However, this is not a case of "conditional sources"; the SRPM will be identical regardless of which release/branch you build it. It would only be "conditional sources" if the %app_subpkg calls or -i flags therein would be conditional, which these are not, nor do I see a need for such. Spec URL: https://yselkowitz.fedorapeople.org/flatpak-app-config.spec SRPM URL: https://yselkowitz.fedorapeople.org/flatpak-app-config-42-1.fc43.src.rpm Description: This package includes configuration files that are installed into select flatpak applications during the building process. (This is intended as a flatpak-specific package, and will only be built to flatpak tags.) Fedora Account System Username: yselkowitz This is more complete, and hopefully gives a better idea of why things are designed as they are. Copr build: https://copr.fedorainfracloud.org/coprs/build/8813225 (succeeded) Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2347045-flatpak-app-config/fedora-rawhide-x86_64/08813225-flatpak-app-config/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. Okay, this package looks fine(ish). Review notes: * Follows packaging guidelines * Licensing is correct * No serious notes from fedora-review and rpmlint PACKAGE APPROVED. The Pagure repository was created at https://src.fedoraproject.org/rpms/flatpak-app-config Thanks. This is a flatpak-specific package, so it won't get any errata of its own; closing. |