search

mate-desktop / mate-panel

MATE panel
https://mate-desktop.org
GNU General Public License v2.0
183 stars 115 forks source link

Use MateImageMenuItem properly #1437

Closed lukefromdc closed 3 months ago

lukefromdc commented 4 months ago

switch out the last vestiges of deprecated GtkImageMenuItem

lukefromdc commented 4 months ago

Depends on https://github.com/mate-desktop/mate-desktop/pull/603 to make MateImageMenuItem subclassable

lukefromdc commented 4 months ago

Travis builds will fail until https://github.com/mate-desktop/mate-desktop/pull/603 is both merged and used by Travis

lukefromdc commented 4 months ago

At this point, my preference if others are OK with it is to release this as mate-panel 1.28.1 depending on mate-desktop 1.28.2 on the grounds that the mate-desktop fix is an emergency fix for a crasher detected at release (due to few testors of our dev versions) and not also updating the panel code leaves iffy code in use for the entire duration of the 1.28 cycle.

This is just my thoughts on it

cwendling commented 4 months ago

@lukefromdc that's also my preference. @raveit65 had issue with bumping dependencies mentioning it made downstream life harder, which I don't really understand but I'm admittedly no downstream maintainer. My counter-argument is the same as yours, but as effectively updating mate-desktop would make things work by itself (although being iffy as you said), I'm not gonna debate that again :)

lukefromdc commented 4 months ago

At this point, I would propose to branch 1.28 and bump versions now, so we can merge this and have a correct fix in master.

We can then do one of two things for 1.28: leave it alone unless and until is breaks in at least one distro due to a GTK change, or cherrypick now. The only thing both distros and users could not do in the latter case is update the panel but not mate-desktop, or roll back mate-desktop but not the panel. If we leave code that works by chance in a release, it works now but is bad code quality and may come back to bite us. A dependency bump six months down the road would be a bigger problem. Short of doing the whole release over as 1.30, we must choose one of the other.

zhuyaliang commented 3 months ago

@lukefromdc How should we handle version release issues? Do I need to merge this PR now

lukefromdc commented 3 months ago

My vote is to merge this, release mate-desktop version 1.28.2, then release mate-panel 1.28.1 depending on mate-desktop 1.28.2 . Though that is unusual, this is an emergency fix for a crash that nobody found until we released and more people started testing, and this correct fix is spread across these two packages.

Otherwise this will be a nuisance throughout the 1.28 lifecycle

raveit65 commented 3 months ago

My vote is to merge this, release mate-desktop version 1.28.2, then release mate-panel 1.28.1 depending on mate-desktop 1.28.2 . Though that is unusual, this is an emergency fix for a crash that nobody found until we released and more people started testing, and this correct fix is spread across these two packages.

Otherwise this will be a nuisance throughout the 1.28 lifecycle

I don't want to jump in again, only a hint..... After releasing mate-desktop-1.28.2 you should update mate-panel CI to build mate-desktop dependency from released 1.28.2 tarball. There are enough examples in git history how to update CI for this step. Otherwise you will broke CI for mate-panel in master.

lukefromdc commented 3 months ago

Oh yeah, thanks for that reminder!

lukefromdc commented 3 months ago

mate-desktop 1.28.2 is out https://pub.mate-desktop.org/releases/1.28/mate-desktop-1.28.2.tar.xz

lukefromdc commented 3 months ago

CI fixed, so this now builds clean at Travis

lukefromdc commented 3 months ago

Should I go ahead and merge this, bump version to 1.29 and them merge to master, or something else? Planning to merge if no objections

lukefromdc commented 3 months ago

Merging, will release 1.28.1 with the same translations as in 1.28.0. Running tx pull -af on a clone of master, then trying to build it netted another broken translation and a build failure:


Error: cannot open mo file ../tzm/tzm.mo
make[3]: *** [Makefile:604: tzm/tzm.stamp] Error 1

Therefore, updating translations has to wait, but this needs to get out the door before more distros first pick up 1.28