https://bugzilla.redhat.com/show_bug.cgi?id=755510 --- Comment #108 from nicolas.vieville@xxxxxxxxxxxxxxxxxxxx --- (In reply to James Hogarth from comment #107) Hello James, First, thank you very much for your review and for sponsoring me for this package. Glad to join the Fedora community. > === Summary === > > A few style things that are non-blockers but I'd suggest taking a look over: > > 1) We no longer use the group field in spec files, this SHOULD be removed: > > https://fedoraproject.org/wiki/Packaging:Guidelines#Tags_and_Sections Corrected. I missed this. > 2) I know there's been a bit of uh questionable and varied version naming > with this package over the years but there but it's sensible to have at > least some link between the rpm version and the upstream version for users > to understand better what they have installed. It helps that you have the > git commit and date, but including the version number that upstream uses as > the release tag is sensible and preferable. > > https://fedoraproject.org/wiki/Packaging:Versioning > > To make the release more readable as well if you are setting a shortcommit > (as indeed you are) in a %global for a snapshot build there's no need to > condition whether it exists in the release field ... it obviously exists as > you just set it and it makes it harder to read. > > If you want to do snapshot builds that's fine... but I suggest the version > should be the most recent tag upstream and then follow the versioning > guidelines for the git tag in the release. To follow this package upstream github repository for a couple of years, I think that version tags set in it are not really relevant. This gnome-shell extension progress through snapshots providing bug correction and new features. I corrected the "versioning" of this package as you suggested above, taking into account that no version at all is provided upstream. Reading the packaging guidelines, this example seems to be correct: Version: 0 Release: 1.20171005%{shortcommit}%{?dist} > 3) Fedora 23 is no longer supported and you can't build for it in koji ... > strip out the "if fedora is 23" conditionals ... they can have no effect now > and just serve to make it messier. Conditionals removed for Fedora 23, as well those for Fedora 24. > As I said these are non-blockers and reflect my personal view of trying to > make the spec a bit tidier and convey the state of the package to users a > bit more clearly, but as the maintainer for this it's your call. You are completely right, I agree with you, the spec file is tidier like that, and easier to read. > Package is APPROVED and you've been sponsored into the packager group. Thank you very much. > Congratulations and welcome :) Glad to join the Fedora community. > The steps to request the repo for your package and building it can be found > here: > > https://fedoraproject.org/wiki/ > Join_the_package_collection_maintainers?rd=PackageMaintainers/ > Join#Add_Package_to_Source_Code_Management_.28SCM.29_system_and_Set_Owner Ok. I will look at this and do the necessary. Thank you again for this review and all the precious advises concerning packaging for Fedora and the unofficial reviews I made. Best regards, -- NVieville -- 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
