Skip to content

Conversation

@tenniscp25
Copy link
Contributor

Description of the change

  • Made spago run call the backend with --run argument.
  • Updated spago run specs for node backend.
  • spago test still uses node backend.

As for the test, if creating a shell script and a batch file to echo $1 to check backend arg is the way to go, I'll be happy to implement it.

Checklist:

  • Added the change to the "Unreleased" section of the changelog
  • Added some example of the new feature to the README
  • Added a test for the contribution (if applicable)

@f-f
Copy link
Member

f-f commented Oct 9, 2019

cc @csicar @andyarvanitis could you take a look at this and maybe try it out?

@csicar
Copy link
Contributor

csicar commented Oct 9, 2019

First of all, thank you for the contribution. It's awesome to see further work being done in that area.

This works properly with pskt. It does start the pskt --run, but I would be awesome if the --run options was passed the module to run. That way, we could also support spago test.

One problem, that remains is, that (at least pskt) does not know where it should look for the foreign files for the standard libraries. That problem is kinda orthogonal to this, but it means, that spago run does will fail for pskt while compiling the jar file, as the foreign files are not found.

Copy link
Member

@f-f f-f left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great, thanks! 👏

The only thing left is to update the fixture for the output when psa is installed (to reproduce locally just install purescript-psa and run the tests)

@tenniscp25
Copy link
Contributor Author

Fixed.

@tenniscp25 tenniscp25 closed this Oct 10, 2019
@tenniscp25 tenniscp25 reopened this Oct 10, 2019
@tenniscp25
Copy link
Contributor Author

Oops.. wrong button 🙈

@f-f f-f changed the title Added support for spago run with alternative backends. (#435) Add support for spago run with alternative backends Oct 10, 2019
@f-f f-f merged commit 531864d into purescript:master Oct 10, 2019
@f-f
Copy link
Member

f-f commented Oct 10, 2019

@tenniscp25 thank you for doing this! 🙂

@andyarvanitis
Copy link

Happy to try this, but it will have to be this weekend. Looking forward to it!

@andyarvanitis
Copy link

Runs fine with psgo; I tested both spago build and spago run. It looks like spago test behaves the same as spago run right now (I assume by design), which is fine as psgo doesn't have anything for test yet.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants