-
-
Notifications
You must be signed in to change notification settings - Fork 159
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
[Obsolete, but see end of thread] Add Combine support and RxSwift extension #311
base: main
Are you sure you want to change the base?
Conversation
@pcantrell There's a new dependency - RxSwift - for tests. Travis is failing but I see you download deps from elsewhere, so I guess you'd need to update that. |
Really excited to take a good look at this! I'm utterly consumed with helping my students with their final projects for another week or so, so please bear the delay in getting to it — but I'm excited to see it when I do. |
No problem. I'm in the process of making RxSwift and Combine copies of the Github browser example, which I'm quite enjoying. I'll add these alongside the existing example. The code I've already committed shouldn't change though, so review without fear if you get to it before I add the examples (which sounds unlikely). |
Sounds fun. Are you using SwiftUI for that? Also, possibly relevant: I'm considering a refactoring that extracts a |
I haven't gone down the SwiftUI path yet. I should get onto that though (in general, not for this project) – I'm starting to feel laggardly rather than prudent. So there'll be a lot in common between the three versions of the example project. Enough that I'll look at having a single project with multiple VC variants and targets rather than create a whole lot of duplication to maintain. Yes, I've created a One thing I did find is that I wanted |
OK, I've added reactive variants to the example project and have made an effort to avoid too much duplication. Same project, separate targets, shared storyboard and API, variants of the main VCs. After saying I wouldn't, I made some changes to the main code too. |
Hi @pcantrell - I've just published a couple of blog posts about the combination of Siesta and Combine/Rx. They're pointing at my fork for now. https://luxmentis.org/swift-reactive-heaven-with-siesta-and-combine/ |
This is really exciting work, Adrian! I have to warn you: the Minneapolis neighborhood that you've seen on the news, the epicenter of this global uprising, is my neighborhood. I live right near the 3rd precinct building that burned down. My head has been almost entirely outside of Work Universe, much less Open Source Work Universe, since the police murdered George Floyd. I am only just now getting back into the regular rhythms, and have quite a backlog. Realistically, I'm going to be very slow to respond to this. Please don't take that slowness as a lack of interest! |
Oh man, I hadn't made that connection. Yes of course - whenever you can. It must be difficult to be optimistic with the general political climate over there, but I hope you get to make some of the necessary changes despite lack of leadership from the top. |
Hey Paul! I hope you're doing well. Could you give an update on the status of this PR? I am considering using Combine with Siesta, but I'd feel much better about putting it in a production app if it was reviewed and in P.S. You never let me know how I could give you an open source bonus to celebrate my app being converted to fully Siesta-based! DM me to let me know. |
Hey @pcantrell, looking for an update on this... Are there plans for this to be merged? What work needs to get this into the main branch? |
Pull requester here. Obviously this is a bit out of date by now, but if @pcantrell is interested I'm still keen to update it and get it merged or discuss implementation etc. I continue to use it in real-world code. |
As do we! I have sent an email to burstoutsolutions, as they seem to not be working on code projects anymore. |
I will keep you posted @luxmentis, and thank you for getting back to me! |
Hey @jdogcoder @jordanpwood @pcantrell and anyone else interested in using SIesta with Combine and SwiftUI, Short version is, this PR is now obsolete, but my new SiestaExt project does all the Combine stuff, plus it adds some nice SwiftUI support and other goodies. Here's a SwiftUI taster to get you interested – multiple resources, plus a loading spinner, error display and a retry button. Note the brevity! (This, combined with the GithubAPI class, is complete working code.) struct MultipleSampleView: View {
let repoName: String
let owner: String
var body: some View {
VStack(alignment: .leading, spacing: 10) {
Text("\(owner)/\(repoName)")
.font(.title)
ResourceView(
GitHubAPI.repository(ownedBy: owner, named: repoName),
GitHubAPI.activeRepositories,
statusDisplay: .standard
) { (repo: Repository, active: [Repository]) in
if let starCount = repo.starCount {
Text("★ \(starCount)")
}
if let desc = repo.description {
Text(desc)
}
Text("In unrelated news, the first active repository is called \(active.first!.name).")
}
Spacer()
}
.padding()
}
} Keen to hear any feedback so please chime in. Bit sad this original PR didn't go anywhere, but I'm still a fan of Siesta even after all this time, hence the new library. I've archived my Siesta fork, but the code is still visible so any apps using it will still work (including a bunch of things I've written). I'm leaving this PR open for the moment so any interested parties can see this news. Although there's technically no reason this sort of code needs to be part of Siesta itself, ultimately (if Siesta has a future) it would be worth at least considering integration into the main repo because discoverability. I shouldn't think anyone's going to think to go looking for a Siesta extension project. As ever, I think Siesta is an underrated piece of work that deserves more attention from the community (and also some more maintenance!). |
Hi Paul,
Here's support for Combine and RxSwift as we talked about. Combine is in the main library code; RxSwift is in Extensions.
It's influenced somewhat by the ReactiveCocoa extension and the talk around that.
As ever, there's more than one way to skin any particular cat, and perhaps especially so in RxSwift. But I've been fairly conservative - nothing too outrageous here.
Feedback entirely welcome of course.
Adrian