Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature request: Add support for --before/--then/--else flags #410

Closed
felixSchl opened this issue Sep 16, 2019 · 6 comments · Fixed by #532
Closed

Feature request: Add support for --before/--then/--else flags #410

felixSchl opened this issue Sep 16, 2019 · 6 comments · Fixed by #532

Comments

@felixSchl
Copy link

felixSchl commented Sep 16, 2019

Pulp has a handful of very useful options to run a command when compilation starts, fails, and succeeds: --before <string>, --else <string>, and --then <string>, respectively. This is useful in combination with --watch in order to, for example, run some post-processing script on the output. Currently the only way to achieve the same is using an external file watcher and run spago build && foobar on every file change. Aside from the additional overhead of getting set up properly for every project, there's also the overhead of cold-starting a build every time (see also: #409, #110) which slows down rebuilds considerably.

@f-f
Copy link
Member

f-f commented Sep 18, 2019

Yeah this sounds reasonable! Though I'd be curious about your specific usecase, to see if we can provide some specific integration before opening up to the full before/then/else workflow

@felixSchl
Copy link
Author

It's a very project specific workflow where we need to run a script over the generated JS to fix up some internal require paths based on the build output. To be honest I have not used --before at all and only discovered it when doing my research for opening this issue, but it seems to make sense to have if we do end up with some sort of --then and --else.

It's not a super urgent feature request by any means because it's really just been a workflow enhancement. That said, if you do think it's a worth while feature to bring in (in terms of maintenance cost going forward,) I'd be happy to take a stab at it.

@StephenWakely
Copy link
Contributor

StephenWakely commented Nov 2, 2019

I used the --then flag to post a notification once pulp had finished building.

pulp --watch --then 'notify-send built' build -O -t ../dist/ps.js

It was quite handy as I didn't have to swap to the terminal to check to see when everything was ready to refresh the browser.

Would love to see this go into Spago.

@f-f
Copy link
Member

f-f commented Nov 4, 2019

Yeah this sounds good, let's go with the way pulp does it since it seems to be a nice design!

@felixSchl
Copy link
Author

I wonder if we can achieve the same thing in a more composable manner. Imagine spago only reporting to stderr during --watch and only reporting build status updates line by line on stdout. Then one could compose quite easily downstream using ordinary shell tooling.

Imagine some structured output like this:

* Build starting
0 Build succeeded
1 Build failed

it would be trivial to do sth like:

spago build --watch | while read -r status; do ...; done

Then again, that precludes spago from cancelling long-running post-build commands. --then to pulp would be canceled when the user triggered a rebuild, which I thought was really useful, too.

@f-f
Copy link
Member

f-f commented Nov 5, 2019

@felixSchl yeah this is also a very interesting approach. Note that starting with #475 logging is starting to move to stderr, so this option is also totally viable (and good by me). On the other hand, the --before/--then/--else approach might be more flexible because of the cancelability aspect

StephenWakely added a commit to StephenWakely/spago that referenced this issue Dec 20, 2019
@f-f f-f closed this as completed in #532 Dec 30, 2019
f-f pushed a commit that referenced this issue Dec 30, 2019
* Add support for `--before`, `--then` and `--else` flags

Fix #410

* Fix before, then, else specs for Windows.

It looks like the space following echo is included in Windows which
was breaking the tests.

* Update README to use a notification example for the before command.

Duplicating the already existing clear-screen flag is redundant.

* If a command fails, it should halt and fail the build.

* The `then` command should run after postbuild.

This would make it run after the bundling process so any
postprocessing on the final bundle can be done.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants