[MAJOR] Feature: pre/post lifecycle

ryan-roemer commented 6 years ago

Breaking: Add pre|post lifecycle commands
Breaking: Error out on invalid or conflicting command line flags passed
Clean up --setup execution and constraints. Add more tests.
Add cross-os functional tests.

coveralls commented 6 years ago

Coverage increased (+0.6%) to 94.257% when pulling 6d71ff0e1849a454eb86599ecce94ff17dce575a on feature/pre-post-lifecycle into 390eaa4618200d20a65752045ba2762e104c70c3 on master.

coveralls commented 6 years ago

Coverage increased (+0.6%) to 94.257% when pulling 6d71ff0e1849a454eb86599ecce94ff17dce575a on feature/pre-post-lifecycle into 390eaa4618200d20a65752045ba2762e104c70c3 on master.

coveralls commented 6 years ago

Coverage increased (+0.6%) to 94.257% when pulling 6d71ff0e1849a454eb86599ecce94ff17dce575a on feature/pre-post-lifecycle into 390eaa4618200d20a65752045ba2762e104c70c3 on master.

coveralls commented 6 years ago

Coverage increased (+1.3%) to 95.008% when pulling f23422a135e6134c5cd9e19688d01d62c73f13e0 on feature/pre-post-lifecycle into 390eaa4618200d20a65752045ba2762e104c70c3 on master.

ryan-roemer commented 6 years ago

@jstrimpel -- Wow, this beast of a "feature" got out of control and caused me to have to clean up a lot of cruft and formerly ambiguous / undefined behavior. I'm almost there with one remaining hurdle to figure out.

Current behavior + complexities is outlined at: https://github.com/FormidableLabs/builder/pull/156/files#diff-04c6e90faac2675aa89e2176d2eec7d8R414

Current status:

builder run <task>: Should work totally fine for pre|post tasks.
builder envs <task>: Not implemented yet, but should be easy -- it's the same case as builder run.
builder concurrent <task1> <task2>: This is where I'm currently figuring things out to make sure the execution semantics make sense. Currently going with: each task takes and execution slot (for limited --queue scenario) and executes its pre, then main, thenpost tasks before being "done" and relinquishing slot.
Lots more unit / functional tests already spec'ed to validate everything works as it should.

I'd say you can kick the tires now if you're just using builder run. Otherwise, it hopefully won't be too much longer before everything is done.

ryan-roemer commented 6 years ago

@jstrimpel -- pre/post should work for all of builder actions now. I've still got to:

Wrangle CI
Add full test coverage for all new scenarios + permutations w/ options.

Let me know if you find any correctness / other issues! Will aim to finish + merge next week.

ryan-roemer commented 6 years ago

Travis and Appveyor seem to be out of git sync with errors like:

 1) bin/builder-core builder run runs a <root>/package.json command:
     Error: Cannot find module './utils/setup'
      at Task.setup (lib/task.js:1:40475)
      at async.series._.extend.tracker (lib/task.js:1:43649)
      at node_modules/async/lib/async.js:718:13
      at iterate (node_modules/async/lib/async.js:262:13)
      at async.forEachOfSeries.async.eachOfSeries (node_modules/async/lib/async.js:281:9)
      at _parallel (node_modules/async/lib/async.js:717:9)
      at Object.async.series (node_modules/async/lib/async.js:739:9)
      at Task.run (lib/task.js:1:43446)
      at Function.invoke (node_modules/sinon/lib/sinon/spy.js:176:59)
      at proxy [as run] (eval at createProxy (node_modules/sinon/lib/sinon/spy.js:81:86), <anonymous>:1:35)
      at Task.execute (lib/task.js:1:51355)
      at module.exports (bin/builder-core.js:1:4717)
      at Context.<anonymous> (test/server/spec/bin/builder-core.spec.js:401:7)

closing and re-opening to hopefully kick. (Just a wild stab in the dark)

ryan-roemer commented 6 years ago

@jstrimpel -- At long last, builder pre|post is all ready to go! I've refactor the entire execution model and improved that along the way too (particularly with handling errors / early exits when things go wrong).

Let me know how it looks on your end. From here, I'm going to do three short additional changes before releasing a new MAJOR:

Add a debug log level.
Default to --quiet=true for toning down all the builder.
Add an engines restriction to node4+ (I won't refactor the code to node4+ now, just want to get the breaking change out of the way)

ryan-roemer commented 6 years ago

Released in builder@4.0.0

FormidableLabs / builder

[MAJOR] Feature: pre/post lifecycle #156