https://bugzilla.redhat.com/show_bug.cgi?id=1529758 --- Comment #3 from Carl George <carl@george.computer> --- This package looks good to me overall, but I have questions/comments about a few minor things. The package requires adwaita-icon-theme, gnome-icon-theme, and hicolor-icon-theme. Why is that? The giturl macro is only used once. That's a pet peeve of mine, so I personally would remove the macro and just insert the url into Source0. Just a suggestion, not required. That said, Source0 doesn't follow the recommend format for GitHub tag tarballs [1]. Please change it to: https://github.com/snwh/%{name}/archive/v%{version}/%{name}-%{version}.tar.gz I think only the LICENSE file should be marked %license [2]. AUTHORS should be marked with %doc. I don't think COPYING should be included, as it appears to just be a brief summary of the actual license. If you still want to include it, it should probably be marked as %doc as well. The scriptlet example in the icon cache guidelines [3] runs touch before gtk-update-icon-cache in %postun, but your %postun just runs gtk-update-icon-cache. After contemplating it, I think the example is incorrect because that directory won't exist at that point. I don't think any action is needed for this, I just wanted to check and see if you understood this the same way I do. [1]: https://fedoraproject.org/wiki/Packaging:SourceURL#Git_Tags [2]: https://fedoraproject.org/wiki/Packaging:LicensingGuidelines#License_Text [3]: https://fedoraproject.org/wiki/Packaging:Scriptlets#Icon_Cache -- 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 _______________________________________________ package-review mailing list -- package-review@xxxxxxxxxxxxxxxxxxxxxxx To unsubscribe send an email to package-review-leave@xxxxxxxxxxxxxxxxxxxxxxx
