Remove dependency on SortedSet #214
Conversation
There was a problem hiding this comment.
Considering the benefits of dropping two non-core dependencies 🎉 . I agree we definitely want to call out in release note.
Since set is ordered in ruby, the interface is still mostly preserved. We also have specs coverage for these, I won't worry too much though 🙂
LGTM 👍
lib/business_time/config.rb
Outdated
| @@ -144,7 +144,7 @@ def weekdays | |||
| wday_to_int(day_name) | |||
| end.compact | |||
|
|
|||
| self._weekdays = SortedSet.new(days) | |||
| self._weekdays = Set.new(days.sort) | |||
There was a problem hiding this comment.
Alternatively,
| self._weekdays = Set.new(days.sort) | |
| self._weekdays = days.sort.to_set |
rmm5t
force-pushed
the
remove-sorted-set
branch
from
6a21b26 to
8c24402
Compare
* develop: Update changelog Fix load path in test suite Update build matrix
|
@kimyu92 Could you please do me a favor, and test this branch on your project(s) that rely on business_time? gem "business_time", github: "bokmann/business_time", branch: "remove-sorted-set" |
That's exactly what I did last night against your branch. It works fine in my project 🙂 |
|
Update: Validated again. It's all ✅ on my end |
|
What would be the timeline to get this in? It would be nice to get an official release 1.0.0 😂 instead of pointing to this branch for production grade project |
|
@kimyu92 I'm planning on testing this with a couple of my own projects today. My original plans were going to make this a v1.0.0 release, but I've since changed my mind. I'm going to make this a v0.12.0 release and wait a few weeks or so to make sure that unsorted After that, I think a v1.0.0 release is warranted for this gem. |
rmm5t
force-pushed
the
remove-sorted-set
branch
from
ef8b56d to
bc878a3
Compare
|
Hi! Any plan on merging this? 🙏 |
I totally forgot about this. Thanks for the nudge. |
|
Woah thank you! |
|
v0.12.0 released. Please note the warnings in the CHANGELOG |
🚨 This is currently an experiment. 🚨
This is an experiment open for discussion and review. I'm not yet sure that this is the best approach, but the discussion in #213 got me thinking. Less external dependencies are usually better, and I'm not sure we were gaining a ton by using
SortedSet. Now thatSortedSetis no longer part of core ruby, these seems like a reasonable trade-off to eliminate its usage.What does it do?
SortedSetWhat else do you need to know?
SortedSetand is no longer part of core Ruby.SortedSetrelies onRBTreeto performSortedSetvs a regularSetRelated Issues