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

Feat/get apps #332

Merged
merged 8 commits into from Jul 25, 2019

Conversation

@Schwartz10
Copy link
Contributor

commented Jul 3, 2019

Changes

Adds a method to get the list of installed applications. @sohkai requested to add this functionality in a separate PR from #328

If you are modifying the external API of one of the modules, please remember to also change the documentation

  • I have updated the associated documentation with my changes

Fixes #194.

@auto-assign auto-assign bot requested review from 2color and sohkai Jul 3, 2019

docs/APP.md Outdated
@@ -108,13 +108,19 @@ const frontendOfApp = new AragonApp(new providers.WindowMessage(window.parent))

Get an array of the accounts the user currently controls over time.

Returns **[Observable](https://rxjs-dev.firebaseapp.com/api/index/class/Observable)**: An multi-emission observable that emits an array of account addresses every time a change is detected.
Returns **[Observable](https://rxjs-dev.firebaseapp.com/api/index/class/Observable)**: A multi-emission observable that emits an array of account addresses every time a change is detected.

This comment has been minimized.

Copy link
@Schwartz10

Schwartz10 Jul 3, 2019

Author Contributor

I snuck in this little grammar change :) But let me know if you want me to remove

@bpierre

This comment has been minimized.

Copy link
Member

commented Jul 3, 2019

Nice! Once merged I’ll update #327 to use it.

@Schwartz10 Schwartz10 referenced this pull request Jul 3, 2019
1 of 1 task complete
@2color

2color approved these changes Jul 3, 2019

Copy link
Contributor

left a comment

LGTM

@sohkai Can you please take a look too?

@Schwartz10 Schwartz10 force-pushed the openworklabs:feat/get-apps branch from 9625922 to 5086937 Jul 7, 2019

@Schwartz10 Schwartz10 force-pushed the openworklabs:feat/get-apps branch from 628902e to 85a7181 Jul 8, 2019

Schwartz10 and others added some commits Jul 10, 2019

@sohkai
Copy link
Member

left a comment

@Schwartz10 I've added a few commits but the most important is 2bd5fce.

In the docs and elsewhere, it seems like the intention was to have a multi-emission observable that emits any time the list of installed apps changes, whereas @aragon/wrapper's getApps() was a sole single-emission observable with the current list of apps.

Let me know if this is correct.

@Schwartz10

This comment has been minimized.

Copy link
Contributor Author

commented Jul 24, 2019

@sohkai thanks for adding some commits here! Multi-emission event should be good - we just need to get the list of all apps, find the Discussion App, and get its proxy address + abi. As long as we can still do that with multi-emission events (which we should), then it seems appropriate.

@sohkai

sohkai approved these changes Jul 25, 2019

@sohkai sohkai merged commit 93aca56 into aragon:master Jul 25, 2019

4 checks passed

WIP Ready for review
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage increased (+1.3%) to 51.582%
Details
license/cla Contributor License Agreement is signed.
Details

@Schwartz10 Schwartz10 deleted the openworklabs:feat/get-apps branch Aug 12, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.