vscode-ext icon indicating copy to clipboard operation
vscode-ext copied to clipboard

Review required and auxiliary apps requirements

Open acuarica opened this issue 2 years ago • 6 comments

We are seeing and increasingly number of issues and PRs due to validation of Required and Auxiliary, dependency, apps, e.g., #66, #71, #74, #117, #136, #137, #147, #165.

Currently the extension declares 5 dependency apps. Currently these are node, git, npm, truffle and ganache. In order to avoid future issues related with these dependencies, we should improve the usage and validation of these apps.

For instance, when git is about to be used, during the execution of the truffle-vscode.newSolidityProject command, the extension validates that node, git and npm are installed. But only git is required at this point. Should change how the validation of required apps works?

Moreover, as pointed out in https://github.com/trufflesuite/vscode-ext/issues/169#issuecomment-1189227989, it might be worth to also evaluate how to generalize the dependency app settings, so we can support #100.

@kevinbluer @xhulz @michaeljohnbennett thoughts?

acuarica avatar Jul 19 '22 14:07 acuarica

Hey @acuarica, thanks for taking care of this 😎

My thoughts:

  • I think we should only validate using --version command or similar and not validate NPM and stuff.
  • Git validation doesn't make sense to me.
  • The validation routine must be performed in the background.

My only concern regarding running in background:

In the past we had problems because some pieces of validation was running without await, running a certain way in background, however, there are several validations running inside the commands. So when the main validation was running in background and someone started a command at the same time, the semserver freaked out and couldn't retrieve the version, displaying the required page from death. That's why, at that time, the quickest remedy I found was to validate that all routines had await, to avoid this evil encounter.

xhulz avatar Jul 19 '22 15:07 xhulz

Hey @acuarica, quite on board with approach! Do you have an estimate in terms of approx effort that might take?

It also feels logical that we consider #100 as we explore this too. E.g. when executing a build, it does a check of the availability of the selected SDK. Only consideration is the potential overhead of doing this each time 🤔

Screen Shot 2022-07-19 at 10 47 35 AM

PS - Was literally just typing this when you're reply came through @xhulz so there may be some redundancy 😅

kevinbluer avatar Jul 19 '22 15:07 kevinbluer

Hi @kevinbluer, thanks for your feedback. I'll edit the description so we include a reference to #100.

Regarding the estimate, I'm not sure how much effort it would take since there is no clear solution yet. Given said that, if I have to provide a rough estimate, I would say around 1 week.

acuarica avatar Jul 19 '22 16:07 acuarica

Hi @xhulz, thanks for your feedback.

  • I think we should only validate using --version command or similar and not validate NPM and stuff.

In case the current project has truffle as a dependency package, should the extension use the global or the local truffle? This answer might help us to figure out how to tackle this.

  • Git validation doesn't make sense to me.

Totally. Moreover, do we want to be a front-end of git as well? Meaning, it is used only to issue a git init command.

  • The validation routine must be performed in the background.

Make sense. I would first refine the approach to validate required apps, and then see if it takes a long time to put it in the background.

acuarica avatar Jul 19 '22 17:07 acuarica

Hey @acuarica, Thanks for your reply!

In case the current project has truffle as a dependency package, should the extension use the global or the local truffle? This answer might help us to figure out how to tackle this.

All references I've been seen about truffle instalation means to use global installation, but I think @kevinbluer and @michaeljohnbennett might be the right people to answer this question

Thank you!

xhulz avatar Jul 20 '22 08:07 xhulz

I think it generally a global package as thats what we recommend but people could in fact install it as a package.json dependency also.

I think to make it easier initially we stick with this process of global installs as there are still sticking points like NPM 6 is the only supported version because of some truffle low level deps.

michaeljohnbennett avatar Jul 21 '22 01:07 michaeljohnbennett