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

Async await guide #435

Merged
merged 2 commits into from
Dec 3, 2018
Merged

Async await guide #435

merged 2 commits into from
Dec 3, 2018

Conversation

ro0gr
Copy link
Collaborator

@ro0gr ro0gr commented Nov 27, 2018

Sharing to track progress on the async/await migration guide.

In addition to the new page It also moves native-events mode page and upgrade instructions from v0 under "migrations/" directory for grouping purposes. Maybe we need a dedicated migrations section in the guides. abondoned it since moving of files would break navigation for the previous versions of guides

One more point to note, we might want to include https://github.com/ro0gr/ember-page-object-codemod to the async/await migration guide. I don't think it's 100% proof solution, however it can be a useful starting point for migration. Tested it here chrislopresto/ember-freestyle#184

@ro0gr ro0gr mentioned this pull request Nov 27, 2018
@coveralls
Copy link

coveralls commented Nov 27, 2018

Coverage Status

Coverage remained the same at 98.762% when pulling ca6b0a8 on ro0gr:async-await-guide into 7fa5913 on san650:master.

---
{% raw %}

If you migrate to `"@ember/test-helpers"` you should take care to update all your "actions" usages to leaverage `async`/`await` syntax.

Choose a reason for hiding this comment

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

If you migrate to "@ember/test-helpers"[,] you should take care to update all your "actions" usages to leaverage leverage async/await syntax.

**Bad**
```js
export default {
scope,

Choose a reason for hiding this comment

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

I find including this unreferenced scope here potentially confusing.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is not only confusing, it's also wrong. Will fix

Copy link

@mistahenry mistahenry left a comment

Choose a reason for hiding this comment

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

one last fix :)

---
{% raw %}

When migrate to use `"@ember/test-helpers"`, you should take care to update all your "actions" usages to leverage `async`/`await` syntax.

Choose a reason for hiding this comment

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

When migrating

@ro0gr ro0gr force-pushed the async-await-guide branch from fc3fd43 to ca6b0a8 Compare December 2, 2018 18:20
@ro0gr
Copy link
Collaborator Author

ro0gr commented Dec 2, 2018

Just updated per @mistahenry feedback.

Also reverted moving of existing migration guides to a separate folder cause it could lead to a links breakage for the previous guide versions.

@mistahenry
Copy link

Looks good to me now

@ro0gr ro0gr merged commit 555518f into san650:master Dec 3, 2018
@ro0gr ro0gr mentioned this pull request Dec 3, 2018
4 tasks
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.

3 participants