`thirdweb deploy` does not respect `--file` option when using `--ci` flag
RyanHolanda opened this issue · 4 comments
When running the thirdweb deploy
command with the --ci
flag and the --file
option, it still gives me the link to deploy contracts that are not in the path specified by --file
.
Example:
thirdweb deploy --ci --file Router
The deployment link will still include contracts that do not have "Router" in the filename.
Solution:
I did a thorough investigation of the code, and it is not filtering the contracts when the --ci
option is passed:
js/legacy_packages/cli/src/common/processor.ts
Lines 193 to 194 in 7dd2869
An easy way to solve this would be to add filtering for the contracts in case the --file
option is passed, similar to how it is done in this section of the code:
js/legacy_packages/cli/src/common/processor.ts
Lines 196 to 203 in 7dd2869
hey @RyanHolanda thanks for the report and thanks for looking into how it could be fixed, massively appreciate that!
normally I'd ask if you're willing to send a PR to fix this. but in this case this would likely be much more involved, our goal is to move these CLI functionalities into a new v5
based CLI, the start of which you can find here
So if you were willing to move all of this in there (which s likely going to be quite the task) - we'd be excited to accept PRs. Otherwise if that seems too daunting I completely understand, and we'll pick this up as part of the planned migration work. (It will likely take a couple of weeks until we get to it however.)
@jnsdls Thanks so much for being open to PRs and allowing us to contribute to the development of thirdweb tools! Unfortunately, I’m pretty busy these days (and likely for the next few weeks). I’d love to dive into contributing to the new v5-based CLI when I have more time, but for now, I’ll have to hold off 🙂
@jnsdls Thanks so much for being open to PRs and allowing us to contribute to the development of thirdweb tools! Unfortunately, I’m pretty busy these days (and likely for the next few weeks). I’d love to dive into contributing to the new v5-based CLI when I have more time, but for now, I’ll have to hold off 🙂
totally understandable, we'll pick this bug up as a part of the CLI migration in the near future!
This will be included as a fix in the v5 CLI migration, closing this for now