-
Notifications
You must be signed in to change notification settings - Fork 200
Conversation
f329b1e
to
6e0faed
Compare
6e0faed
to
120b678
Compare
Small comment before reviewing, I'd love to have the CLI working by default and add a |
As discussed offline, let's tackle that as a separate issue |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, nice job @spalladino!
@@ -179,9 +163,11 @@ export default class NetworkAppController extends NetworkBaseController { | |||
return []; | |||
} | |||
|
|||
const ownedProxies = proxies.filter(proxy => !proxy.admin || proxy.admin === this.appAddress); | |||
// TODO: If 'from' is not explicitly set, then we need to retrieve it from the set of current accounts |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
or fail... can we add an issue to track this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this.networkFile.app = { address } | ||
} | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we split this into multiple files?
Adds a
--light
flag to the init command to create a lightweight app using a SimpleProject. Dependency management is to be implemented later.To reviewers: I strongly suggest to review this PR with whitespace diff hidden, and preferrably by-commit.
Depends on #161
Fixes #74