thirdweb-dev/js

`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:

if (options.ci) {
selectedContracts = compiledResult.contracts;
.

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:

const filtered = compiledResult.contracts
.filter((c) => {
if (typeof options.file === "string" && options.file.length > 0) {
return c.fileName.includes(options.file);
}
return true;
})

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