via ir build #22
via ir build #22
Conversation
This comment was marked as outdated.
This comment was marked as outdated.
QGarchery
requested review from
MathisGD,
a team,
adhusson,
Rubilmax,
MerlinEgalite and
Jean-Grimal
and removed request for
a team
There was a problem hiding this comment.
- tests work with Metamorpho compiled with the foundry config of the Public allocator
- and forge build --size skips Import, so Blue and Metamorpho such that it doesn't throw an error
just imo in the readme we could write that in the tests metamorpho isn't the same as the one deployed onchain for this reason (it's ok but let's write it somewhere, as it could cause discrepancy in gas test or something)
| @@ -30,10 +30,10 @@ jobs: | |||
| - name: Run Forge build | |||
| run: | | |||
| forge --version | |||
| forge build --sizes | |||
| forge build --sizes --skip Import | |||
There was a problem hiding this comment.
Skipping Import could be added to a foundry "build" profile so the whole build config has a single source of truth (i understand it may be overkill for now because there's only 1 option and 1 place where it's used)
As a new dev here, i would forge build ; is it what's expected?
Otherwise we could argue that having a build profile makes it clearer how this repository is intended to be built
There was a problem hiding this comment.
It's the same in the end, all will be built the same way. Skipping imports just delays the build of dependencies to when actually running tests (delaying is only useful here because we check the sizes)
There was a problem hiding this comment.
forge build builds correctly the public allocator, so that's ok for me
Continuation of #21