-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
👷♂️👷♀️RxSwift 5 Pre-Release discussion #1921
Comments
@kzaher the purpose is mainly to see which of these we want to do for RxSwift 5, as long as we're making breaking changes here. |
Maybe |
@valerianb Great point. I don't think we should entirely remove it, but we probably should move everything from runtime deprecations to hard deprecation (e.g. |
Oh I thought there was a similar mechanism already in place with |
Hi @freak4pc, I think that these are small changes we can add.
If you can help with list on top, that would be great.
I think we can deprecate
I think we should also include this. This one is probably the biggest and requires intimate knowledge of the internals so I can take that one.
I've responded to that thread, I think we can close this one. Not sure there is an action point on it. |
I can take over the top list, no problem. I’m also ok with closing the Completable one- we might be able to tackle this at a later release, I’d leave that to your discretion. |
@freak4pc Would love to have the compactMap PR in 5. |
It's in the list (look at the end) ;-) |
Ummmm.... I could have sworn.... ;) |
@freak4pc I've refactored schedulers. I don't think there is any more confusion with take and count. |
I mentioned on the other thread that I still think it's worth adding a label for one of them to differentiate. Types alone aren't enough in my personal opinion. |
I think that the last blocker for 5.0 is #1799. |
We won't be able to release this for 5.0 because of the Xcode 10.2 Swift 5 issue I've mentioned. We should release without it. Should we also leave the |
This is swift https://developer.apple.com/documentation/swift/array/1689487-prefix It doesn't have label for count and we don't have a label for time operators. I think that the problem was worse before when you could specify a constant which could mean two different overloads. Right now it should be more clear. I'm personally fine with the current state. Adding a label to count would be against Swift prefix API, and if we add label to time version we would need to add that label everywhere for consistency. I'm not even sure what label would we use. |
Sounds good. I think this concludes the RxSwift 5 content we discussed unless you have any more thoughts. |
@kzaher Actually prefix has multiple overloads and anything aside from the "count" one has a label .. |
@freak4pc I know, that's what I've posted that link.
|
Ah, I misunderstood. I was also referring to labeling the duration overload, not the count one. I though of either |
I think that You look at this as a decision for a single operator. I look at this as a decision to go through all temporal operators and decide new labels for each of them and deprecate the old ones also. Without extremely convincing reason I would not want to do all of that. |
Don't really understand what you're referring to with stdlib, feel free to share with us. The concrete examples are everywher, just look for |
I‘m currently only on my iPad, I‘ll check later. But in general I would prefer a stdlib like coding style, that makes things easier to read (that is ofcourse my opinion). func foo<AnObservable>(
_ observable: AnObservable
) -> Observable<AnObservable.Element>
where
AnObservable: ObservableType
{
...
} That ofcouse won‘t be rendered by the ASTPrinter like that when the user cmd + click on the API in Xcode. That aside Sent with GitHawk |
Yup, it's definitely a personal opinion. I don't mind either style but I actually find the style where the protocol of the constraint is inline more readable. Personal preference for sure :) |
I've solved Is there anything else we want to do in the public interface? |
I think that takes care of only the public API - for the rest I already did the other APIs as well just to be thorough. Do you want to leave that for now? Or should we go with |
Maybe we should use |
I just realized one interesting fact. From SE the basic direction for generics UI goes into Sent with GitHawk |
If and when that SE would actually happen, we can have another discussion. Even though to me "Some" is superfluous. |
Sure :) My point was that Sent with GitHawk |
I think this should be everything for 5.0, or do we have something else? |
I can’t think of anything else, personally. |
I'm publishing 5.0, and everything went ok until .... http://cocoapods.org/pods/RxRelay Maintained by Wassim Seifeddine. God damn it. |
@kzaher the repository is dead for 2 years and it's basically doing exactly the same we want to do. @wassimseif can we find an agreement and reclaim the cocoapods spec namespace for |
Getting the privileges from @wassimseif would be optimal. Otherwise we can reach to the CocoaPods team and see if they'll be willing to help (I've seen several scenarios they were able to) |
I've had to delete 5.0.0 of RxSwift because of this :( |
I've sent an email to @wassimseif. I guess one thing is sure, this won't be released today probably :( Damn it. |
I also e-mailed him haha :) |
Any everyone pinged him here at least three times. 🤞 |
haha :) true. |
Well, we had some bad experiences with name camping. Somebody has https://github.com/RxSwift organization and didn't want to transfer it a while ago :) That's why we moved the project to ReactiveX :) |
The good thing about the repository is that it's dead and it's doing what we're doing here now. :) |
Actually think being part of ReactiveX is much nicer and more confident-inducing anyways :] |
Pod name ownership was transferred successfully, we're good to go @kzaher ! 🎉 |
@freak4pc I'll do it tonight, I need to change pod description because cocoapods were also complaining about that. |
Sounds good. Once that's done you might want to do |
This is done.
We should never delete any tags. |
Hey @kzaher & others -
Since RxSwift 5 is around the corner, I took the liberty of making a list of PRs/Issues that were delayed for RxSwift 5.
The goal here is to see if:
Since this is a major version bump we should take some time to think about any breaking changes we might want to introduce.
This is what I gathered so far:
DispatchTimeInterval
instead ofTimeInterval
for Schedulers. Crash when convert Double to Int #1472 @kzahercombineLatest
of an empty array completes immediately.combineLatest
of an empty array completes immediately #1879toArray
to returnSingle
. Return Single trait for toArray() method #1923 @freak4pcVariable
, globalRxTest
things) @freak4pc #19223compactMap
Add CompactMap to RxSwift #1925. @hmlongcotake
amount vstake
interval ambiguity. @freak4pc (blocked by Crash when convert Double to Int #1472)resultsSelector
missing closure labels for some overloads of combineLatest & zip.Completable.merge
in favor ofCompletable.zip
. Add Completable.zip (alias for Completable.merge) #1929 Deprecate Completable.merge #1931Generic Renames Checklist:
CompatibleType
toReactiveBase
#1947SubjectType.SubjectObserverType
toSubjectType.Observer
Rename S to Subject, SubjectObserverType to Observer #1950If there are any other PRs/Issues you feel should participate in this discussion, please add a comment below.
The text was updated successfully, but these errors were encountered: