chore: update foundry version in anvil impl #275
Conversation
suchapalaver
force-pushed
the
suchapalaver/update-foundry-version-in-anvil-impl
branch
from
58b04ec to
761be3c
Compare
| const TAG: &str = "stable@sha256:daeeaaf4383ee0cbfc9f31f079a04ffb0123e49e5f67f2a20b5ce1ac1959a4d6"; | ||
| const TAG: &str = "latest@sha256:d44ade9e940d6a4ac420e6d07da3bcecccbeeaf7d662ed0a3791c7c9713c93bc"; |
There was a problem hiding this comment.
We usually prefer not to update versions unless there is a clear need (e.g., a new incompatible version is released or the current version is unmaintained). Additionally, different users may use different versions.
Moreover such updates must be considered as breaking changes as they may introduce unexpected behavior changes.
Here, as far as I can see, there are no changes except for the version itself. Therefore, I assume users can pin their desired version using ImageExt::with_tag in their code.
Is there another reason for this update besides the version change?stable@sha256:daeeaaf4383ee0cbfc9f31f079a04ffb0123e49e5f67f2a20b5ce1ac1959a4d6 still works fine?
There was a problem hiding this comment.
I think we should allow for the idiosyncrasies of the different images we work with. In this case the simplest thing would be to pull in the latest image since the project moves quickly. I get the concern with breaking changes but we are already seeing this issue with breaking tests in unrelated features. Would you be open to updating the image to simply latest on this anvil feature? That would create the best user experience.
There was a problem hiding this comment.
User can just set latest in their own code. That's their decision what version to use. And only 1 line of code.
And if it stops working for them - they can prepare a PR with new incompatible changes. But it won't affect others immediately just because of frivolous default version.
But it won't be good if someone's setup(with the defaults we provide) will be broken just because of underlying update of image.
Another issue of latest here is also lack of proper ownership. Who gonna be responsible to fix the module of latest tag doesn't work anymore? Sometimes it requires understanding of the component and how to configure it properly.
We must avoid flakiness and don't increase it. Otherwise we won't be able to distinguish what is expected behavior and what's not. I vote for deterministic approach
There was a problem hiding this comment.
Just to clarify, if user wants latest - it's really simple:
let node = AnvilNode::default().with_tag("latest").start().await?;@suchapalaver
But I can suggest alternative approach:
We can add method latest to AnvilNode which will use latest tag by default:
let node = AnvilNode::latest().start().await?;This is just a shortcut, but I believe it's nice UX for users based on your experience
And the same time - default won't be flaky due to this. It's explicit choice of user
The anvil tool is part of the larger Foundry suite of blockchain development tools. Foundry just a major release so I've updated the image version in our anvil implementation.