libgweather is a library to access weather information from online services for numerous locations. libgweather3 is a version of libgweather for use with GTK+ 3. Spec: http://mclasen.fedorapeople.org/libgweather3.spec Srpm: http://mclasen.fedorapeople.org/libgweather3-2.91.0-1.fc15.src.rpm When this is approved, I will change libgweather to also depend on libgweather-data (needed to avoid file conflict between the packages).
Suggestions:- 1) I see this package includes versioned BuildRequires. Can you add some comments why is it needed to have versioned BR: ? I think as we used to first build required build dependencies in required release, there should not be any need to write versions. 2) For usage of obsoletes and provides see http://fedoraproject.org/wiki/Upgrade_paths_%E2%80%94_renaming_or_splitting_packages#Do_I_need_to_Provide_my_old_package_names.3F this package added Obsoletes: gnome-applets-devel < 1:2.21.4-1 but I see gnome-applets-devel-2.21.4-1 was built for FC9. So, this obsolete is not needed. 3) Is there any bug reference for Patch0: ? 4) Please we have got updated Gconf2 scriptlet snippet. Good to use that. See 5) Any reason to turn off verbose output in %posttrans? What does gtkdocize do?
for GConf scriptlet snippet, https://fedoraproject.org/wiki/Packaging/ScriptletSnippets#GConf
Created attachment 452894 [details] spec cleanup
(In reply to comment #1) > Suggestions:- > > 1) I see this package includes versioned BuildRequires. Can you add some > comments why is it needed to have versioned BR: ? > I think as we used to first build required build dependencies in required > release, there should not be any need to write versions. At least the gtk3 version is somewhat useful, as the package won't build against 2.90.x > this package added > Obsoletes: gnome-applets-devel < 1:2.21.4-1 Huh ? I see no Obsoletes in the spec. > 3) Is there any bug reference for Patch0: ? No, its the same patch that is in the libgweather package > 4) Please we have got updated Gconf2 scriptlet snippet. Good to use that. See True > 5) Any reason to turn off verbose output in %posttrans? People complain if scriptlets are verbose
(In reply to comment #4) > (In reply to comment #1) > > Suggestions:- > > > > 1) I see this package includes versioned BuildRequires. Can you add some > > comments why is it needed to have versioned BR: ? > > I think as we used to first build required build dependencies in required > > release, there should not be any need to write versions. > > At least the gtk3 version is somewhat useful, as the package won't build > against 2.90.x > Ok. If you need it you can keep it. But please remove others. > > > this package added > > Obsoletes: gnome-applets-devel < 1:2.21.4-1 > > Huh ? I see no Obsoletes in the spec. There it happens mismatch. Sometimes it happens srpm contains some other spec and SPEC url shows some other spec contents. I can still see obsoletes in spec packaged in srpm. I see no issues then as it looks spec url shows no obsoletes. > > > 3) Is there any bug reference for Patch0: ? > > No, its the same patch that is in the libgweather package Ok, but good if some bug reference would have given for that patch. > > > 4) Please we have got updated Gconf2 scriptlet snippet. Good to use that. See > > True > > > > 5) Any reason to turn off verbose output in %posttrans? > > People complain if scriptlets are verbose But good to make it verbose so that people can see if anything goes wrong in executing that scriptlet. If you still think -q should be used then no problem use it.
Spec: http://mclasen.fedorapeople.org/libgweather3.spec Srpm: http://mclasen.fedorapeople.org/libgweather3-2.91.0-2.fc15.src.rpm I've dropped most library versions, and the -q from %posttrans
Please can you tell me any reason for using old Gconf2 scriptlet snippet? Also, add missing Requires(pre): GConf2 Requires(post): GConf2 Requires(preun): GConf2 I can review and approve this now but unfortunately, there are many packages still using old GConf2 scriptlets and now this new package is also using old snippet.
No particular reason, just forgot. I can fix it during the import, if thats good enough for you.
Suggestions: 1) Add BR: gobject-introspection-devel 2) Add following to -devel Requires: %{name} = %{version}-%{release} 3) Add following to libgweather-data %defattr(-,root,root,-) 4) Use the correct GConf2 scriptlets 5) Good to preserve timestamps in make install command 6) Is there any reason not to create libgweather3-data? Good to add some comment in spec to have subpackage name as libgweather-data APPROVED.
> 6) Is there any reason not to create libgweather3-data? Good to add some > comment in spec to have subpackage name as libgweather-data The only reason I did it this way is that it would be a little odd if libgweather depended on libgweather3-data. Thanks for approving. I'm out of the country this week, will get to importing this (with the requested fixes) when I'm back next week.
ping
One month passed since this review is approved. time to needinfo on reporter.