-
-
Notifications
You must be signed in to change notification settings - Fork 376
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
refactor(list)!: remove deprecated start_corner
and Corner
#759
refactor(list)!: remove deprecated start_corner
and Corner
#759
Conversation
Perhaps we could let this one live for a minute? There's not a huge amount of users, but 0.25 is the most recent release. |
I think one version is enough for an easy fix like that. But I don't mind holding it for a while. Let's see what the others have to say. Maybe we should state in the contributing guide how much we want to wait before removing deprecated items. |
To add to my previous comment: When the deprecated feature was clear and didn't have problem in itself, I agree to keep it a few version. |
I think 2 releases is a good idea generally. This gives our users enough time to update to a release, test on that release and have users test it for a bit. We're releasing Ratatui about every 6-8 weeks, so a 2 version deprecation gives 3 months notice of breaking changes. There might be a few changes that we want to move users away from more forcefully so we can implement a new feature where the 2 release approach doesn't work, but for things like this, where the downside of keeping it is only that the API is messy, I'm fine with it. ^Weak opinion - if you do feel more strongly than I do about removing this sooner, then I'm happy to support that. |
Weak opinion for me too, It's literally that I don't like keeping a messy API for too long. So conclusion for me:
|
Updated CONTRIBUTING.md |
+1 from me, I think having a 2 release notice is good enough for removing deprecated items. Let's also add these types of issues to the future milestones (which I saw it is already added!). |
Let's merge this right before 0.27.0 releases to avoid putting breaking changes in the repo right after release. Marking it draft until then. |
`List::start_corner` was deprecated in v0.25. Use `List::direction` and `ListDirection` instead.
c1949c3
to
08f796b
Compare
Rebased on main. It is unlikely we will release 0.26.2 in my opinion. |
Probably not, but it's also not a huge thing to hold back from merging. Merging this means that we can't do 26.2 if needed. |
Works for me, was easy to rebase and probably still will be later on. We can wait. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #759 +/- ##
=======================================
- Coverage 94.3% 91.9% -2.4%
=======================================
Files 61 61
Lines 14767 15668 +901
=======================================
+ Hits 13926 14401 +475
- Misses 841 1267 +426 ☔ View full report in Codecov by Sentry. |
Is Corner used somewhere else? Otherwise, it can be removed too. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
needs a CI fix, otherwise LGTM
Nope - removing. |
start_corner
start_corner
and Corner
…ui#759) `List::start_corner` was deprecated in v0.25. Use `List::direction` and `ListDirection` instead. ```diff - list.start_corner(Corner::TopLeft); - list.start_corner(Corner::TopRight); // This is not an error, BottomRight rendered top to bottom previously - list.start_corner(Corner::BottomRight); // all becomes + list.direction(ListDirection::TopToBottom); ``` ```diff - list.start_corner(Corner::BottomLeft); // becomes + list.direction(ListDirection::BottomToTop); ``` `layout::Corner` is removed entirely. Co-authored-by: Josh McKinney <[email protected]>
List::start_corner
was deprecated in v0.25. UseList::direction
andListDirection
instead.layout::Corner
is removed entirely.