-
Notifications
You must be signed in to change notification settings - Fork 254
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): start_corner
is now direction
#673
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #673 +/- ##
=====================================
Coverage 90.9% 90.9%
=====================================
Files 42 42
Lines 12678 12710 +32
=====================================
+ Hits 11526 11558 +32
Misses 1152 1152 ☔ View full report in Codecov by Sentry. |
8e37ce4
to
2b0f8ae
Compare
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.
To make this more of a user friendly change, instead of renaming the existing setter method and enum, keep both and mark them deprecated with a note on the replacement. Make the implementation of the old code call translate the corner to direction. This allows consuming libraries to be notified of changes as a warning in their build process rather than breaking immediately.
87924d3
to
9ee0f91
Compare
Rebased on #672 |
6ed3d0c
to
8edb67e
Compare
Right I completely forgot about the depreciation warning. I added back I'm wondering if this is still a breaking change then? |
If we keep the old behavior as deprecated, I don't think so. |
start_corner
is now direction
start_corner
is now direction
8edb67e
to
fd41c10
Compare
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.
2 Small things. A small simplification to new() and another about how we want to communicate breaking changes.
Otherwise this is great.
The previous name `start_corner` did not communicate clearly the intent of the method. A new method `direction` and a new enum `ListDirection` were added. `start_corner` is now deprecated
fd41c10
to
ffa91c3
Compare
The previous name
start_corner
did not communicate clearly what the function was doing. The method was renamed todirection
and a new enumListDirection
was added.Fixes #671
Same as #672, I feel like this should be for 0.26.