https://bugzilla.redhat.com/show_bug.cgi?id=1948659 --- Comment #2 from Ray Strode [halfline] <rstrode@xxxxxxxxxx> --- (In reply to Kalev Lember from comment #1) > The first thing that sticks out to me is that source rpm is named > gnome-kiosk-40.alpha-1.fc34.src.rpm while the spec file goes to great > lengths to use ~alpha instead of .alpha. Looks like something has gone wrong > with it if the srpm file is named like this? heh. Missing slashes on the my sed line. Dunno how I missed that. > In other gnome packages we've used a system where we put this at the top of > each file: > > %global tarball_version %%(echo %{version} | tr '~' '.') > > and then used %{name}-%{tarball_version} in the Source line and in the > %setup line, which then automatically translates 40~alpha to 40.alpha for > those two places. Maybe it would be easiest to use the same system here? See > e.g. > https://src.fedoraproject.org/rpms/gnome-shell/blob/rawhide/f/gnome-shell. > spec I kind of like keeping the upstream version at the top, and encoding from it, since it seems more "canonical" to me. > Another thing that sticks out is that none of the C code files in the > tarball have license headers. Would be nice to add them -- although I don't > think it's a requirement. That was actually intentional. Those top blurbs are redundant, get stale, end up listing the wrong copyright holders, or just a subset of copy right holders, might have the wrong address in them etc. I figured since this is a new project I'd just do away with them. > Some in line comments about the spec file: > > > #VCS: git:git://gitlab.gnome.org/halfline/gnome-kiosk > > I think you can just take the VCS line out, it was for Colin's old > automation that nobody uses any more. wfm. > > > %package -n gnome-kiosk-search-appliance > > Summary: Example search application application that uses GNOME Kiosk > > License: GPLv2+ > > Requires: %{name}%{?_isa} = %{version}-%{release} > > You can't use %{?_isa} in a noarch package -- it's going to be different on > different arches, but noarch means that it's supposed to work on all arches. oops. I actually added the BuildArch: noarch at the last minute when I was rereading the spec, and missed this. will fix. > %files -n gnome-kiosk-search-appliance > %{_datadir}/applications/org.gnome.Kiosk.SearchApp.desktop > %{_datadir}/gnome-session/sessions/org.gnome.Kiosk.SearchApp.session > %{_datadir}/xsessions/org.gnome.Kiosk.SearchApp.Session.desktop > %{_datadir}/wayland-sessions/org.gnome.Kiosk.SearchApp.Session.desktop > > Looking at the files list, it sounds like the package should have runtime > requires on gnome-session to work? indeed. > Should the package have appdata to show up in gnome-software? I don't think it makes sense to show up in gnome-software. Maybe it should be NoDisplay=true even. -- 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 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 on the list, report it: https://pagure.io/fedora-infrastructure
