Overview
Sorry to keep you waiting, and thanks for the extra hands on this one.
Let's talk about improvements/corrections.
The first - bullet point you wrote GTK3, but the NEWS file says:
+NetworkManager-strongswan-1.6.0 +------------------------------- + +- Support for GTK 4 +- Removed libnm-glib compatibility +
And we should always add a bullet point for changes in dependencies like this:
BuildRequires: pkgconfig BuildRequires: pkgconfig(gtk+-3.0) >= 3.0 +BuildRequires: pkgconfig(gtk4) BuildRequires: pkgconfig(libnm) BuildRequires: pkgconfig(libnma) >= 1.1.0 +BuildRequires: pkgconfig(libnma-gtk4) BuildRequires: pkgconfig(libsecret-1)
Since we have GTK 4 support now, there's no need to keep pkgconfig(gtk+-3.0) around
while adding pkgconfig(gtk4). We should be replacing the former with the latter,
instead of just adding the latter. And don't forget to mentioned the addition of
pkgconfig(libnma-gtk4).
So you should add new - bullet points with something along the lines:
- Update to version 1.6.0 + Support for GTK3 + Removed libnm-glib compatibility - Add pkgconfig(foo) as BuildRequires because of this. - Replace pkgconfig(bar) with pkgconfig(baz) because of that.
This way the changes file get as much self contained as possible when/if someone need to track down something that changed a while ago.
Sorry for not answering, kind of forgot about this. Gonna open a new request with the fixes applied.
Request History
KaratekHD created request
- Update to version 1.6.0
+ Support for GTK4
+ Removed libnm-glib compatibility
- Add pkgconfig(gtk4) because the new version requires this
- Add pkgconfig(libnma-gtk4) because the new version requires this
gnome-review-bot accepted review
Check script succeeded
luc14n0 accepted review
LGTM
luc14n0 approved review
LGTM
luc14n0 accepted request
Accepting.
Thanks for your time.
GTK3 is still required for the build to succeed.
OK. We may try to patch GTK 3 out, then.
For now thanks for your time.
OBS: I'm going to accept this, even though there are two trailing white spaces being introduced (I'll remove them myself). But try to avoid this is future submissions, please (that's valid for any devel project).
Alright, thank you for the constructive feedback!
One thing to keep in mind: What if other GTK based desktop environments want to use this? If they don't support GTK4 in their settings yet patching out GTK3 might break them.
Right. That's something to keep in mind. However, one package depending on both GTK 3 and 4 is something I don't like. We may end up using multibuild flavors to build each versions, if possible.