cashapp / redwood

Multiplatform reactive UI for Android, iOS, and web using Kotlin and Jetpack Compose
https://cashapp.github.io/redwood/0.x/docs/
Apache License 2.0
1.68k stars 73 forks source link

Do a generic type parameter cleanup pass #1586

Open JakeWharton opened 1 year ago

JakeWharton commented 1 year ago

We had one of these a while ago but it has devolved a bit. WidgetApplier.kt is a mess, for example.

I think we should do:

We also could do AppT, ViewT, and WidgetT to really be unambiguous and free up single-letter ones to be whatever, especially in the light of the next sentence.

Otherwise prefer T. We have some Rs for return value and Es for elements. There's also a P for a Widget.Provider subtype. The lint tooling has a function with Items, Keys, and Groups.

ahmed3elshaer commented 8 months ago

@JakeWharton should we go across all generics in the repository or just WidgetApplier.kt ?🙈

JakeWharton commented 8 months ago

I think the whole repository needs this

ahmed3elshaer commented 8 months ago

@JakeWharton I think it's essential, too. I will update my draft PR to this scope.

On another subject: can I contribute to any open issue, or should I stick to the pr-welcome label?

JakeWharton commented 8 months ago

If you're doing the whole repo, I would consider only doing one type parameter at a time (such as all the ones which represent view type) in individual PRs. Otherwise if all are done at once I suspect it will be such a large PR that it will be impossible to review in detail.

JakeWharton commented 8 months ago

And yes, any issue is fair game. If it doesn't have PR welcome it usually just means that it's larger in scope.

ahmed3elshaer commented 8 months ago

If you're doing the whole repo, I would consider only doing one type parameter at a time (such as all the ones which represent view type) in individual PRs. Otherwise if all are done at once I suspect it will be such a large PR that it will be impossible to review in detail.

That is helpful, I will follow this 👌

ahmed3elshaer commented 8 months ago

And yes, any issue is fair game. If it doesn't have PR welcome,it usually just means that it's larger in scope.

Thanks for the clarification, I would love to see contribution guidelines. It can help and encourage people to participate