https://bugzilla.redhat.com/show_bug.cgi?id=2275304 Siteshwar Vashisht <svashisht@xxxxxxxxxx> changed: What |Removed |Added ---------------------------------------------------------------------------- Flags| |needinfo?(msuchy@xxxxxxxxxx | |) --- Comment #11 from Siteshwar Vashisht <svashisht@xxxxxxxxxx> --- SPEC: https://download.copr.fedorainfracloud.org/results/packit/openscanhub-openscanhub-261/srpm-builds/07340017/osh.spec SRPM: https://download.copr.fedorainfracloud.org/results/packit/openscanhub-openscanhub-261/srpm-builds/07340017/osh-1.0.0.20240423.152702.g044fd77.pr_261-1.src.rpm PR: https://github.com/openscanhub/openscanhub/pull/261/files Please review the changes directly there. > Issues: > ======= > - Permissions on files are set properly. > Note: See rpmlint output > See: https://docs.fedoraproject.org/en-US/packaging- > guidelines/#_file_permissions I fixed some of them. Others are needed to keep the current deployment working. Please see comments on the spec file. > - If (and only if) the source package includes the text of the license(s) > in its own file, then that file, containing the text of the license(s) > for the package is included in %license. > Note: License file LICENSE-SELECT2.md is not marked as %license > See: https://docs.fedoraproject.org/en-US/packaging- > guidelines/LicensingGuidelines/#_license_text This is from a dependency. I would skip listing it. > - systemd_post is invoked in %post, systemd_preun in %preun, and > systemd_postun in %postun for Systemd service files. > Note: Systemd service file(s) in osh-worker, osh-hub > See: https://docs.fedoraproject.org/en-US/packaging- > guidelines/Scriptlets/#_scriptlets It is invoked as expected. > - license file must be installed Fixed. > - there is no owner of /etc/osh directory It is owned by `osh-common` package. > Should: > - note that usually the order of section is %prep, %build, %install, all > scriptlet, all %files section. Your style of mixing %files and scriptlets > make it hard for me to read the spec file. I could take that as a separate issue. I would avoid doing it now to make it easy to do reviews. > - when using tarball from GH then you should specify full URL in URL tag. > See > https://docs.fedoraproject.org/en-US/packaging-guidelines/SourceURL/ > #_tag_example It was removed by PackIt. It should be fixed now. > - the tarball has test, can it be run in %check? I could take that as a separate issue. > - instead of cp -R you want to use cp -aR (and I think the R is not needed > at all). Fixed. -- You are receiving this mail because: You are on the CC list for the bug. You are always notified about changes to this product and component https://bugzilla.redhat.com/show_bug.cgi?id=2275304 Report this comment as SPAM: https://bugzilla.redhat.com/enter_bug.cgi?product=Bugzilla&format=report-spam&short_desc=Report%20of%20Bug%202275304%23c11 -- _______________________________________________ package-review mailing list -- package-review@xxxxxxxxxxxxxxxxxxxxxxx To unsubscribe send an email to package-review-leave@xxxxxxxxxxxxxxxxxxxxxxx Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedoraproject.org/archives/list/package-review@xxxxxxxxxxxxxxxxxxxxxxx Do not reply to spam, report it: https://pagure.io/fedora-infrastructure/new_issue
