search

gtk-rs / gtk

DEPRECATED, use https://github.com/gtk-rs/gtk3-rs repository instead!
https://gtk-rs.org/
MIT License
1.25k stars 82 forks source link

Remove unneeded wrappers #1048

Closed GuillaumeGomez closed 4 years ago

GuillaumeGomez commented 4 years ago

cc @sdroege @EPashkin

sdroege commented 4 years ago

:+1:

GuillaumeGomez commented 4 years ago

Updated!

fengalin commented 4 years ago

Shouldn't we deprecate them with a message hinting to use the glib variants?

sdroege commented 4 years ago

The docs already mention that, and it's a trivial change that can be automated with sed

GuillaumeGomez commented 4 years ago

And it'll be mentionned in the release notes because we're nice people. :)

fengalin commented 4 years ago

I don't mind, but my point was to use a language feature that makes it automatic to point users in the right direction when they upgrade their dependencies, without the need to look in any doc.

We can still do it C-old-school style though ;)

GuillaumeGomez commented 4 years ago

We can do that but it means that'll have to track them...

sdroege commented 4 years ago

We can do that but it means that'll have to track them...

And we did that in gstreamer-rs once and only today I noticed that the deprecated functions were not removed yet, more than a month after release. It's additional maintenance burden.

We can still do it C-old-school style though ;)

You can also do that in C. Maybe we should do a bugfix release that deprecates the functions now on crates.io already?

fengalin commented 4 years ago

That or just launch a [rip]grep every now and then to remove previously deprecated items.

I find it very nice to get a warning for such a thing when I upgrade my dependencies in mass. I don't necessarily browse all the release notes for all crates.

fengalin commented 4 years ago

And we did that in gstreamer-rs once and only today I noticed that the deprecated functions were not removed yet, more than a month after release. It's additional maintenance burden.

Sorry, that was my idea. I should have checked with you how long we would keep them deprecated before removing them and created an issue to track their removal.

I still believe this is nice to users though.

GuillaumeGomez commented 4 years ago

How do you differentiate between the not-yet deprecated items (meaning, not released as such yet) and the others?

fengalin commented 4 years ago

I agree there's some tracking involved at one point. You can indicate the release from which deprecation occurs. I think deprecated items can stay for a couple of releases. It might help people still get the hint should they skip one or two releases. Then we can follow a principle such as: after n releases after deprecation, the item is removed, which tracking is obvious thank to the depreciation release attribute.

sdroege commented 4 years ago

While it seems like the right thing to do and is possible in this case, it's much harder or even impossible to add deprecations in other cases as the old API can't really be kept.

GuillaumeGomez commented 4 years ago

Two things: first I opened a PR to examples to fix the CI issue. Second, I propose to make a small release with the deprecation notice (which will be in a commit only in the corresponding branch) so that everyone is satisfied: no need for us to keep track of anything and users who are up-to-date will have the deprecation notice.

GuillaumeGomez commented 4 years ago

Reminder: still needs to add the deprecation marker on the current release and make a small release with it.

GuillaumeGomez commented 4 years ago

Usual mac failures so let's finally move on.