pcaversaccio / createx

Factory smart contract to make easier and safer usage of the `CREATE` and `CREATE2` EVM opcodes as well as of `CREATE3`-based (i.e. without an initcode factor) contract creations.
https://createx.rocks
GNU Affero General Public License v3.0
304 stars 18 forks source link

🛠 Add Tests for `internal` Methods #9

Closed mds1 closed 1 year ago

mds1 commented 1 year ago

🕓 Changelog

🐶 Cute Animal Picture

Put a link to a cute animal picture inside the parenthesis-->

pcaversaccio commented 1 year ago

Btw, I didn't ask at all, is the PR already for review?

mds1 commented 1 year ago

Yes definitely! My process is to open a PR as draft if it's not ready for review, and take it out of draft once ready (or open it not in draft when ready from the start). Let me know if there's another approach you want to use

pcaversaccio commented 1 year ago

That's completely fine. I just asked because it's a private repo and draft PRs require a GH pro version (which I don't have). Will do a proper review tomorrow/Sunday.

mds1 commented 1 year ago

draft PRs require a GH pro version

Ah did not realize that, good to know

pcaversaccio commented 1 year ago

draft PRs require a GH pro version

Ah did not realize that, good to know

Just for private repos, for public ones anyone can do it.

mds1 commented 1 year ago

Everything looks good overall! Good call adjusting the specs. I might tweak them a bit and update the test structure accordingly on monday—the solidity file structures are autogenerated from the spec using bulloak -w ./**/*.tree, so I want to make sure we're still conforming to that. This does sometimes result in unused modifiers if you don't need any special setup in them, but typically for this BTT approach we keep the modifiers as separators/self-documentation on the spec.

Soon we'll also have a bulloak check command to verify that the tests and spec match.

pcaversaccio commented 1 year ago

Cool, sounds good to me 👍

pcaversaccio commented 1 year ago

Everything looks good overall! Good call adjusting the specs. I might tweak them a bit and update the test structure accordingly on monday—the solidity file structures are autogenerated from the spec using bulloak -w ./**/*.tree, so I want to make sure we're still conforming to that. This does sometimes result in unused modifiers if you don't need any special setup in them, but typically for this BTT approach we keep the modifiers as separators/self-documentation on the spec.

Soon we'll also have a bulloak check command to verify that the tests and spec match.

If you're done with your update, just re-request my review before merging.