-
Notifications
You must be signed in to change notification settings - Fork 212
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
Introducing UIKit & Cocoa extensions in RxSwitExt (e.g. RxCocoaExt) #124
Comments
@freak4pc I just gave a first reading to this issue and
Secondly we must apply a strict rule that this is
Swiftlint is welcomed I really like it so it should be a rule
I will stick will camel-case because it creates more code harmony and most of us are coming from
I totally agree but there should be a way to test what is going in.
I totally agree to this also. Finally I would suggest we go for a git workflow where |
I don't mind either way, I'm easy. I think most users don't mind either, there's not enough volume of code in the single RxSwiftExt repo to qualify as "bloat" -- a separate repo would mean one more pod, and one more framework in applications, which adds to the startup time. Aside from this, a separate repo could make discovery easier for users (but then maybe it's just a matter of building good documentation). THIS SAID. Some of the points raised are perfectly valid. Regardless of whether we make a separate repo, there are better practices we could put in place right now that would help improve quality (which is already high, IMHO) and ease interaction with contributors.
Yes possibly. I usually don't use such tools as I think all they do is fulfil a narcissistic vision of the code, but why not. I'm not oppose to having a linter if it makes code easier to read for everyone.
And for good reason. Operators are not classes, and they are camel-cased in RxSwift itself. I don't see what uppercasing would bring aside from confusion. There are things NOT camel-cased in RxSwift (i.e. test file names), I'll fix this. We need consistency, I agree.
Yes! Totally worth it.
Not sure about this one. The main issue with Playground (especially when it comes to UI playgrounds) is that they are unreliable. OTOH, it's easy and relatively painless to make a page with a quick demo in a playground, getting straight to the point in a contained example. A full-fledged application is harder to come by (if you want to have one app to show all features).
Totally favoring the proposal-first approach. I highlighted steps for RxSwift in CONTRIBUTING.md document I recently updated there, I think we should follow a similar discipline. I really appreciate the exceptional attention to detail that you @freak4pc put into your PR reviews. It bumped the standard of what a proper review should be, as you tend to go very deep in the details. It's probably going to be difficult to keep the same level of attention with more reviewers, but this should be part of the "best effort" towards quality. |
@fpillet I did mention about a fresh repo because it would be totally be a focused concept of realizing all the things that one would need when working on
@fpillet let's say that I find it really hard with playgrounds sometime, but this is not a conclusion that it doesn't work. I think we can defer this for now. @freak4pc @fpillet if all good I can set-up the repo with |
Thanks @freak4pc for the detailed issue, there's lot to tackle here. If you don't mind, I'd like to break this down into 3 parts with potential to generalize a couple of them to the overall RXSwiftCommunity organization.
I think this is a very good seed for an organization-wide Vision or Manifest document. I believe it will be useful to have such a document describing the vision and motivation behind RxSwiftCommunity without guidelines or rules directly included but rather to be derived from if I'd say.
Again, I can see a benefit from generalizing this to be organization wide -with few exceptions of course-. If agreed, I'd suggest starting a team discussion for each item of those and try to settle on a greatest common divisor across all repos and gradually apply that. Moving to RxSwiftExt specific matters :) regarding how to introduce RxCocoaExt; ;TLDR; I am totally against the first option, while I think the other two options are equally good with the second is better from users perspective (but adds burden to maintainers) and third is simplest as it builds on existing conventions.
|
Thanks everyone for all your opinions :) I'll try to address everything The other two approaches are too prone to issues:
Regarding the caveat you mentioned @mosamer - I don't think it's a huge deal. But TBH you don't have to do this. They are separate specs, so you could have separate versioning, but it would make it much harder to reason for and track so you probably wouldn't want to 😉 @fpillet regarding some of your points
It's definitely high. But my long-term ambition is to get MANY more people involved... and to make that effort scalable we need to assume we're going to get some junk PRs, so the more we're standardized, the easier this is gonna be :P
I get where you're coming from but the repetitively of commenting on trivial code styling issues is pointless and spends precious (and limited) contributor time. I'm all up for automation to try and reduce this stress so we can concentrate mainly on code and functionality reviews 🥇 We can try it out and see if it's a pain or not, and adjust as we go.
That's exactly what I wrote the next line 😄 I just wanted to confirm, and yes having uniformity is what I would aim for in general. Operators should be camelCase where if you build an entirely new class you would probably want to make it ClassCase.
That's true. Actually no problem with enforcing Playgrounds for every PR / new addition 👍
Really really appreciate the kind words, especially coming from you 🙏 I'm going to compile a list of some Action Items so we don't discuss this to much and actually just GTD ! 💪💪💪💪💪 |
Continuing Action Items discussion at #127 |
There has been increasing activity and demand for a place where UI-Related components and extensions could live in the RxSwiftCommunity eco-system.
This would help:
After a lot of discussion inside the community and with @fpillet , it seems there are three options here.
The last two options seem best to keep things consolidated, and also have more community involvement since this would be a place where more contributors could be involved. but this is one of the reasons for this discussions.
If we do either option we need to discuss several things here:
mapTo
) instead of Class-case (e.g.MapTo
). I guess because its operators vs. classes in this case, but we should make a decision on what makes sense and how to name files across different boundaries. (same for Folder structure here)I'm sure there are many more points to discuss but this would be a great starting point.
I hope to get involvement from @RxSwiftCommunity/contributors, when and if you have some time.
Thanks all!
Shai 🎉
The text was updated successfully, but these errors were encountered: