Use latest (experimental) versions of wrapper & api #1249
Conversation
chadoh
force-pushed
the
update-client-to-0.8-experimental
branch
3 times, most recently
from
1caf54e
to
3638d4e
Compare
There was a problem hiding this comment.
We definitely should freeze this feature for now and focus into the actual web3 integration before trying to add these new experimental features.
I spent some time last weekend testing the latest @aragon/cli version and it has some serious issues with some contracts that I don't see them being addressed here, the @aragon/cli updating should be a separate PR on its own.
Please avoid merging this without further testing/discussion or explicit approval.
I don't think this features represent a blocker item to continue working into the web3 integration tasks planned for this cycle.
chadoh
force-pushed
the
update-client-to-0.8-experimental
branch
9 times, most recently
from
1788052
to
4bcea2f
Compare
ghost
removed their request for review
aragonCLI needs to be installed as a global package in order to work
correctly. This updates the README accordingly, and updates the `aragon`
commands to work with the new version:
• Use aragen address from latest CLI release in PlanningKit.sol
• Pass `--propagate-content false --skip-confirmation true`, which are
now required for familiar v5 functionality
• Pipe 'yes' into 'aragon publish' commands. The new version of
aragonCLI waits for user input saying this:
→ ? Couldn't reuse artifact due to mismatches, regenerate now? [y]es/[a]bort
Long-term, we need to submit a patch to aragonCLI that allows
automatically saying "yes" to this prompt, rather than waiting for
user input. For now, we can pipe `yes` into it, which accomplishes
what we want, but results in ugly logs.
* use @aragon/api from https://github.com/AutarkLabs/aragon-api * point aragonCLI at experimental fork of aragon client, AutarkLabs/aragon#27 * this client uses our own experimental version of @aragon/wrapper, https://github.com/AutarkLabs/aragon-wrapper * Replace `api.cache` calls with `api.trigger`, which uses the still-experimental `trigger` interface under development at aragon/aragon.js#361
There's no way to run `aragon devchain` that makes it return to the terminal with the chain running in the background. Since it never returns, there's no good & quick way to run these two commands back-to-back. Instead, this gets around the problem most of the time by giving the devchain ten seconds to start up before continuing.
chadoh
force-pushed
the
update-client-to-0.8-experimental
branch
from
4bcea2f
to
8bd084a
Compare
| Node.js LTS or greater required. | ||
|
|
||
| - Note: @aragon/cli and truffle npm deps are automatically installed when bootstrapping. | ||
| * Node.js LTS or greater required. You can use a tool like [`asdf`](https://asdf-vm.com/) or [`nvm`](https://github.com/nvm-sh/nvm) to manage versions of node. |
There was a problem hiding this comment.
That seems too obvious, and maybe adding engine field to package.json would help since we are not supporting every node version from LTS right now, at least we didn't test them all, we are in fact using 10.x branches in places like CI, so this statement seems not really accurate.
Said that I like the suggestions about the node versions managers, I didn't know about asdf myself, so I liked this message... I let to you the final decision on how to make up this based on my comment @chadoh or just keeping it as is, if you consider that.
There was a problem hiding this comment.
Oh, gotcha, yeah, that makes sense. I was keeping the comment that had been there somewhat intact, "Node.js LTS or greater required" – I would have preferred to not change it at all, for now. Adding engine to package.json is on my list of things to do; maybe we can improve this line and add that in a follow-up PR?
| @@ -38,6 +38,7 @@ start_testrpc() { | |||
| aragon devchain --reset --port "$testrpc_port" & | |||
| elif [ "$DEV" = true ]; then | |||
| aragon devchain --reset --port "$testrpc_port" & | |||
| sleep 10 # wait for devchain to start TODO: modify cli to return rather than require interruption | |||
There was a problem hiding this comment.
No the most efficient way to do, but if that works, for now should be enough...
Hopefully we will get rid of this whole script sooner rather than later
There was a problem hiding this comment.
Exactly! I added similar comments to the commit message where I did this.
There was a problem hiding this comment.
It seems much better!
I guess the other aragon apm commands are meant to launch manually, so keeping the confirmation there makes sense.
Due to our imperative needs of having this merged, I couldn't try locally to extensively check things like the hardcoded addresses passed into the script, and the stores working with these changes, but I trust you guys telling me this was tested by multiple people.
From the code review perspective it seems nice for me!
Great job @chadoh
| @@ -16,8 +16,8 @@ | |||
| "precommit": "lint-staged", | |||
| "prepublishOnly": "truffle compile --all", | |||
| "publish": "aragon apm publish patch && sleep 3", | |||
| "publish:cd": "aragon apm publish major --files dist/ --environment continuous-deployment --apm.ipfs.rpc https://ipfs.autark.xyz:5001 --ipfs-check false", | |||
| "publish:http": "npm run build:script && aragon apm publish major --http localhost:1111 --http-served-from ./dist", | |||
| "publish:cd": "yes | aragon apm publish major --files dist/ --environment continuous-deployment --apm.ipfs.rpc https://ipfs.autark.xyz:5001 --ipfs-check false --skip-confirmation true", | |||
There was a problem hiding this comment.
I guess if we add the skip confirmation option we shouldn't need the yes solution, but it shouldn't harm to have both for now, so why not?
There was a problem hiding this comment.
We do need both, unfortunately. With only --skip-confirmation true, it still gets stuck at this step:
Couldn't reuse artifact due to mismatches, regenerate now? [y]es/[a]bort
I consider it a bug in aragonCLI that --skip-confirmation doesn't apply here, and I'd like to see one of us submit a patch soon.
api.cachecalls withapi.trigger, which uses the still-experimentaltriggerinterface under development at API: event trigger (#361) aragon/aragon.js#361Read commit comments for more details.