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

x/mobile/app: export the App instance #12872

Closed
rakyll opened this issue Oct 7, 2015 · 3 comments
Closed

x/mobile/app: export the App instance #12872

rakyll opened this issue Oct 7, 2015 · 3 comments
Milestone

Comments

@rakyll
Copy link
Contributor

@rakyll rakyll commented Oct 7, 2015

The app package requires the following boilerplate and a closure provided to app.Main.

func main() {
    app.Main(func(a app.App) {
        // ...

Are there any cases where we will support multiple app.App instances? If not, why don't we just export an App instance from the app package. It may have an accessor that blocks until the app instance is ready.

func main() {
   a := app.App()
   for e := range a.Events() {
       // ...
   }
}

The lack of a globally accessible app instance leads third party packages polluting their APIs. A typical example is the x/exp/sensor package where the Enable function needs to accept a sender implementation in order to dispatch the events to the app event channel. Within the same app, there must be a single App instance and user should never be exposed to third party APIs that requires it as an argument.

/cc @crawshaw @hyangah

@rakyll rakyll added this to the Unreleased milestone Oct 7, 2015
@hyangah
Copy link
Contributor

@hyangah hyangah commented Oct 7, 2015

Sorry, I am missing something. How does the change in the app package help exp/sensor package avoiding the sender argument?

Some apps may want to collect and aggregate sensor data using the sensor package but independently of the app package. They may choose to occasionally send the aggregated data to the app.Main's f but, through an independent channel. (for { select { case <- a.Events() , case <- sensorChannel ... } } )
I hope less packages to depend on the app package, so that more packages in mobile repo become available to Gobind apps.

@rakyll
Copy link
Contributor Author

@rakyll rakyll commented Oct 8, 2015

Some apps may want to collect and aggregate sensor data using the sensor package but independently of the app package.

The rationale behind the API rewrite was to enable APIs that dispatches events through the app event channel. If we don't desire to couple any package with the app package, our rewrite attempt is fundamentally wrong. The previous APIs and implementations were completely app package agnostic and were less error prone.

If the intention is to decouple the app package, I'd rather revert all the changes to the sensor package and let the users decide whether they would like to pipe the events to the app event channel or not themselves.

@rakyll
Copy link
Contributor Author

@rakyll rakyll commented Oct 8, 2015

Considering @hyangah's feedback about reducing the dependencies to the app package, I am closing this issue. The sensor redesign discussion is off-topic under this issue.

@rakyll rakyll closed this Oct 8, 2015
@golang golang locked and limited conversation to collaborators Oct 9, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants
You can’t perform that action at this time.