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

Ensure beforeRun is included in command instrumentation. #6628

Merged
merged 2 commits into from Jan 1, 2017

Conversation

rwjblue
Copy link
Member

@rwjblue rwjblue commented Jan 1, 2017

Prior to this change command.beforeRun was being included in init instrumentation. This change fixes that by moving .beforeRun into command instrumentation.

This change also ensures that beforeRun can return a promise (which we had previously documented in the API docs but was not actually supported until this commit) and adds a test.

When calling `beforeEach` / `afterEach` like this these blocks are
ran for **every** test in **all** files.  That is **_bad_**!
Prior to this change `command.beforeRun` was being included in `init`
instrumentation. This change fixes that by moving `.beforeRun` into
`command` instrumentation.

This change also ensures that `beforeRun` can return a promise (which
we had previously documented in the API docs but was not actually
supported until this commit) and adds a test.
@rwjblue rwjblue mentioned this pull request Jan 1, 2017
5 tasks
@rwjblue rwjblue requested a review from Turbo87 January 1, 2017 17:14
Copy link
Member

@Turbo87 Turbo87 left a comment

Choose a reason for hiding this comment

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

👍 on the first commit. I'm not familiar with the code in the second one.

@rwjblue
Copy link
Member Author

rwjblue commented Jan 1, 2017

@homu r+

@homu
Copy link
Contributor

homu commented Jan 1, 2017

📌 Commit 13faa4a has been approved by rwjblue

@homu
Copy link
Contributor

homu commented Jan 1, 2017

⌛ Testing commit 13faa4a with merge 8def5eb...

homu added a commit that referenced this pull request Jan 1, 2017
Ensure `beforeRun` is included in `command` instrumentation.

Prior to this change `command.beforeRun` was being included in `init` instrumentation. This change fixes that by moving `.beforeRun` into `command` instrumentation.

This change also ensures that `beforeRun` can return a promise (which we had previously documented in the API docs but was not actually supported until this commit) and adds a test.
@homu
Copy link
Contributor

homu commented Jan 1, 2017

☀️ Test successful - status

@homu homu merged commit 13faa4a into ember-cli:master Jan 1, 2017
@homu homu mentioned this pull request Jan 1, 2017
@rwjblue rwjblue deleted the before-run-in-command branch December 19, 2019 17:00
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.

None yet

3 participants