Skip to content
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

Renames ui modules to workflow-ui-core, workflow-ui-android. #239

Merged
merged 1 commit into from
Mar 30, 2019

Conversation

rjrjr
Copy link
Contributor

@rjrjr rjrjr commented Mar 29, 2019

No description provided.

@rjrjr rjrjr requested a review from zach-klippenstein March 29, 2019 22:43
Copy link
Collaborator

@zach-klippenstein zach-klippenstein left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Excellent!

import com.squareup.workflow.ui.ViewBinding
import com.squareup.workflow.ui.ViewRegistry
import com.squareup.workflow.ui.backstack.ViewStateStack.Direction
import com.squareup.workflow.ui.buildScene
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is com.squareup.workflow.ui.android.* too long a package name? It would be nice if the package names corresponded to the module names.

Copy link
Contributor Author

@rjrjr rjrjr Mar 29, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not so much that it's too long as that it's kind of an orthogonal concern, kind of redundant. Is that extra package name really useful to the android developer, or would it be cognitive clutter?

I did the same thing in the sample code -- the coordinators in the android module are in the same packages as the related screen objects in the common module.

WDYT?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll merge this as-is, and follow up if you feel strongly.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yea, I don't feel strongly. And if you're writing a UI for non-android, you're not going to depend on both android and javafx (e.g.) artifacts anyway.

zach-klippenstein added a commit that referenced this pull request Mar 29, 2019
`WorkflowHost` is the name of the class that is the entrypoint to the runtime,
but this module actually contains the entire runtime implementation, and
so I think it's a more accurate name.

Inspired by #239.
@zach-klippenstein zach-klippenstein added this to the v0.11.0 milestone Mar 29, 2019
@zach-klippenstein
Copy link
Collaborator

You've inspired me to do #240.

@rjrjr rjrjr changed the base branch from ray/version-names to master March 29, 2019 23:04
@rjrjr rjrjr force-pushed the ray/android-workflow-ui branch from e7f3c44 to b0d125e Compare March 29, 2019 23:25
@rjrjr rjrjr force-pushed the ray/android-workflow-ui branch from b0d125e to 6dc9967 Compare March 29, 2019 23:30
@rjrjr
Copy link
Contributor Author

rjrjr commented Mar 29, 2019

Updates made unintentional reformat intentional / better, and rebased to keep up w/master

@rjrjr rjrjr merged commit 5b83f0d into master Mar 30, 2019
@rjrjr rjrjr deleted the ray/android-workflow-ui branch March 30, 2019 00:02
@zach-klippenstein zach-klippenstein added the kotlin Affects the Kotlin library. label Mar 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kotlin Affects the Kotlin library.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants