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

Port ember-cli/broccoli-builder to master #360

Closed
oligriffiths opened this Issue Apr 3, 2018 · 6 comments

Comments

Projects
None yet
2 participants
@oligriffiths
Copy link
Contributor

oligriffiths commented Apr 3, 2018

I am going to attempt to summarize the work that needs to be done to port the changes made to https://github.com/ember-cli/broccoli-builder into broccoli, so that the fork can be retired and ember-cli can use broccoli master

Glossary:

Diff:

I created a patch from branch-point-with-tests to 0-18-x oligriffiths/broccoli-builder#7 that shows the changes basically from the whole 0-18-x series so we can see what has actually changed (minus file deletions and the conversion of the test suite from tap to mocha).

Todo:

  • On 0-18-x, port test suite to mocha from branch point + tests, as this happened just after the branch point, so to make comparisons easier I have ported the test suite to use mocha
  • On master, add the ability to overwrite a dist directory without having to remove the directory first #354
  • On master, port the protection of the dist directory from -0-18-x if it's a direct parent of the project #358
  • On 0-18x deprecate api_compat.js. This was to support the old read API, and is no longer supported in Broccoli and hasn’t been used in ember for some time. Start with a console warning, then remove. https://github.com/oligriffiths/broccoli-builder/blob/diff/lib/api_compat.js ember-cli/broccoli-builder#26
  • On 0-18x kill api_compat.js.
  • On master, port heimdall stats collection from 0-18-x Investigate the event api in Broccoli for this instead of hand jamming: #362
  • On master, port cancellation from 0-18-x #350
  • On master port broccoli-build-error from 0-18-x https://github.com/oligriffiths/broccoli-builder/blob/diff/lib/broccoli-build-error.js this is a nicer error output that only shows the stack trace for where the error happened within a Broccoli plugin
  • On master validate the ability to have the tmp directory in the project root (per ember-cli) for compat with addons that have used this hack
    ember-cli/ember-cli#7798

Bonus points

  • Mid-build change events should cancel the current build and queue up behind the existing cancel before kicking off another build. This allows for if a watched file changing mid build, to cancel the current build and kick off a new one. This is out of scope for upstreaming to master. #350
@oligriffiths

This comment has been minimized.

Copy link
Contributor Author

oligriffiths commented May 20, 2018

Ok, here's the state of play:

Heimdall support: #362
Broccoli-1 support in ember-cli ember-cli/ember-cli#7798 - this includes tmp support in project root
The last bit is port broccoli-build-error from 0-18-x https://github.com/oligriffiths/broccoli-builder/blob/diff/lib/broccoli-build-error.js but I don't think this is super important for landing 1.0 support in ember-cli

@stefanpenner

This comment has been minimized.

Copy link
Contributor

stefanpenner commented Jun 11, 2018

@oligriffiths what about the watcher-middleware? I think unifying them (if they are not already) is important. Specifically: https://github.com/ember-cli/broccoli-middleware/blob/master/lib/watcher-middleware.js#L30

Which enables: ember-cli/rfcs#80

@oligriffiths

This comment has been minimized.

Copy link
Contributor Author

oligriffiths commented Jun 11, 2018

@stefanpenner I think we should do that as a separate step as it’s not directly part of Broccoli-builder and the intention here is to remove the builder.

@oligriffiths

This comment has been minimized.

Copy link
Contributor Author

oligriffiths commented Jun 11, 2018

To clarify, as part of the existing builder work no, as part of the quest, yes

@stefanpenner

This comment has been minimized.

Copy link
Contributor

stefanpenner commented Nov 29, 2018

@oligriffiths can you confirm but I believe this has landed right?

@oligriffiths

This comment has been minimized.

Copy link
Contributor Author

oligriffiths commented Nov 29, 2018

Yup

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