search

DeLaGuardo / setup-clojure

GitHub Action to provision clojure's most popular build tools for Linux, Mac OS X and Windows.
MIT License
183 stars 27 forks source link

Install clojure cli tools to a location common to powershell & pwsh #62

Closed ikappaki closed 2 years ago

ikappaki commented 2 years ago

Hi,

could you please consider patch to install the Clojure cli tools to a common PowerShell modules location that is discoverable by both the powershell and pwsh shells. It addresses #61.

This is done by setting the PSModulePath variable to the common location before installing the Clojure cli tools on MS-Windows. The Clojure cli tools install script looks into that variable to present the user with the list of paths to install to, and this action chooses option 1, i.e. this path.

I've generated the js script code by running npm run all, and I've also updated the examples in smoke tests and the readme file, not sure if there are more references.

The smoke tests should pass once committed to master, not sure how to run them from the branch.

Thanks

DeLaGuardo commented 2 years ago

Looks like it is still failing smoke test Could it be because exec is running a new powershell here? I'm not a windows user so it is hard for me to reason why it is happening

ikappaki commented 2 years ago

Looks like it is still failing smoke test Could it be because exec is running a new powershell here? I'm not a windows user so it is hard for me to reason why it is happening

It is failing because the smoke test in my PR branch references the master branch which doesn't have the fix. Would you like to change the smoke testa action to reference the PR branch so that it passes, and before we are about to merge I switch the reference back to master?

Thanks

DeLaGuardo commented 2 years ago

Looks like it is still failing smoke test Could it be because exec is running a new powershell here? I'm not a windows user so it is hard for me to reason why it is happening

It is failing because the smoke test in my PR branch references the master branch which doesn't have the fix. Would you like to change the smoke testa action to reference the PR branch so that it passes, and before we are about to merge I switch the reference back to master?

Thanks

right, sorry, I should check that first :) yes, please check with your repo/branch first

DeLaGuardo commented 2 years ago

Amazing! I see it works now.

ikappaki commented 2 years ago

right, sorry, I should check that first :) yes, please check with your repo/branch first

Done, could you please have another look to confirm?