Fedora Account System
Red Hat Associate
Red Hat Customer
Spec URL: https://trix.fedorapeople.org/rocm.spec SRPM URL: https://trix.fedorapeople.org/rocm-6.3.3-1.fc43.src.rpm A meta package to make it easier for the user to install the ROCm packages at once. Reproducible: Always
Copr build: https://copr.fedorainfracloud.org/coprs/build/8706892 (succeeded) Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2348762-rocm/fedora-rawhide-x86_64/08706892-rocm/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.
Any reason you made a libs subpackage instead of just using "rocm"? I.e. rocm and rocm-devel instead of rocm-libs and rocm-devel? Technically some of those aren't libs, and usually a libs subpackage is only used if you want a subdivision for people who want less than everything. E.g. llvm vs llvm-libs
Spec URL: https://trix.fedorapeople.org/rocm.spec SRPM URL: https://trix.fedorapeople.org/rocm-6.3.3-1.fc43.src.rpm I am not sure why I went with -libs now, maybe because the AMD release had it. Going with plain rocm package now. Also Cleaned up devel requires. Added a -test subpackage, now just for kfdtest.
Created attachment 2080215 [details] The .spec file difference from Copr build 8706892 to 8768646
Copr build: https://copr.fedorainfracloud.org/coprs/build/8768646 (succeeded) Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2348762-rocm/fedora-rawhide-x86_64/08768646-rocm/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.
Small gripe, can you please add a better descriptio for the devel package? Also you licensed the package as GPL. That's probably ok, but since this is a metapackage, shouldn't it match the fedora default license (CC BY-SA 4.0)? I'm not 100% sure what metapackages usually have.
Spec URL: https://trix.fedorapeople.org/rocm.spec SRPM URL: https://trix.fedorapeople.org/rocm-6.3.3-1.fc43.src.rpm I changed the description for devel,test to be similar to the main package. For the license, I changed it to MIT because most of the ROCm packages are MIT and looking around for what to license it I found https://docs.fedoraproject.org/en-US/legal/fedora-linux-license/ And seemed appropriate.
Created attachment 2080315 [details] The .spec file difference from Copr build 8768646 to 8770520
Copr build: https://copr.fedorainfracloud.org/coprs/build/8770520 (succeeded) Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2348762-rocm/fedora-rawhide-x86_64/08770520-rocm/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.
Yeah MIT seems safe, it's a metapackage, so not much to say there. The style looks fine, packages install fine. Shouldn't devel require the base package though? Or maybe that's just a redundant require?
It is a redundant require.
Approved
The Pagure repository was created at https://src.fedoraproject.org/rpms/rocm