morpho-org / morpho-optimizers

Core contracts of Morpho V1 Optimizers.
https://app.morpho.org
GNU Affero General Public License v3.0
137 stars 22 forks source link

Export script #1611

Closed QGarchery closed 1 year ago

QGarchery commented 1 year ago

Fixes:

This simple script allows you to export env variables in the same way the makefile does it. This way you can edit the command you want to use (in your terminal), without needing to change a versioned file.

:warning: The script exports env variables in the current shell, meaning that subsequent calls to make or forge will use those variables. It should be fine because variables defined in .env.local will override those if you run make later. If you don't want to change variables in the current shell, you can always create a new shell in one of the following ways:

  1. use ( . ./export_env.sh && forge test )
  2. use bash and then . ./export_env.sh followed by your commands such as forge test and then exit to return to the parent shell and clear the env variables

Left to do: factorize the script and the makefile preambule (no idea how to do it yet, since make does not allow to run scripts in the preambule)

MathisGD commented 1 year ago

Where's the script ?

QGarchery commented 1 year ago

Where's the script ?

Sry, I just added it

QGarchery commented 1 year ago

Maybe this PR is OK as it is now, and it would be a bit overkill to factorize setting the environment variable. What do you think @morpho-dao/solidity, should we merge this ?

QGarchery commented 1 year ago

It does not work for me, because of the @config remappings

It works fine for me with the @config remappings. Can you paste the output of ( . ./export_env.sh && forge test ) please @MathisGD ?

MathisGD commented 1 year ago

I was doing ./export_env.sh && forge test which does not work.

But it still doesn't work, like if the chain was not forked (and I don't see the RPC url being defined in your script)

MerlinEgalite commented 1 year ago

Tbh this is not a prio to change this since this repo will be less and less updated. Perhaps it's not worth spending time on this

QGarchery commented 1 year ago

But it still doesn't work, like if the chain was not forked (and I don't see the RPC url being defined in your script)

Yes sorry, I forgot the most important part. It should be fixed now.

I was doing ./export_env.sh && forge test which does not work.

I found that the most convenient is to do bash and then . ./export_env.sh and then you can call forge directly as many times as you want

MathisGD commented 1 year ago

CI is failing for a rounding error, otherwise LGTM !

MathisGD commented 1 year ago

Please take a look at the failing test before merging