adopted-ember-addons / ember-cli-flash

Simple, highly configurable flash messages for ember-cli
https://www.npmjs.com/package/ember-cli-flash
MIT License
355 stars 113 forks source link

CHORE: Updates ember-try and code ready #308

Closed abhilashlr closed 4 years ago

abhilashlr commented 4 years ago
abhilashlr commented 4 years ago

@sbatson5 Seems like the tests are PASS. I am still unsure if run.later is the right way to solve the component integration test case.

sbatson5 commented 4 years ago

Thanks for all the work @abhilashlr. I left one more comment.

Also, we try to maintain a clean git history with 1 commit per PR (when possible). Merge commits are actually disabled, meaning we prefer to squash and merge. When updating a branch, we prefer rebasing for that reason, rather than merging master in, as it obscures the history a bit.

Not a huge deal, but for future PRs, rebasing would be preferred.

abhilashlr commented 4 years ago

Merge commits are actually disabled, meaning we prefer to squash and merge.

Squash commits would merge all of them into 1 commit, isn't it?

Not a huge deal, but for future PRs, rebasing would be preferred.

I made 3 PRs at a time and messed up the commits in my fork. Should be fine in the next set of PRs if any👍 .