Skip to content

[Feature] util blueprint for module unification#16490

Merged
mixonic merged 1 commit intoemberjs:masterfrom
daibhin:dn/mu-utils-blueprint
Apr 27, 2018
Merged

[Feature] util blueprint for module unification#16490
mixonic merged 1 commit intoemberjs:masterfrom
daibhin:dn/mu-utils-blueprint

Conversation

@daibhin
Copy link
Contributor

@daibhin daibhin commented Apr 11, 2018

@daibhin daibhin force-pushed the dn/mu-utils-blueprint branch from d1b05fd to 68c9641 Compare April 11, 2018 18:30
@daibhin daibhin changed the title [WIP][Feature] utils blueprint for module unification [WIP][Feature] util blueprint for module unification Apr 11, 2018
@daibhin daibhin force-pushed the dn/mu-utils-blueprint branch 3 times, most recently from d037383 to bed42ae Compare April 11, 2018 19:40
@daibhin daibhin changed the title [WIP][Feature] util blueprint for module unification [Feature] util blueprint for module unification Apr 11, 2018
@daibhin daibhin changed the title [Feature] util blueprint for module unification [WIP][Feature] util blueprint for module unification Apr 11, 2018
@daibhin daibhin force-pushed the dn/mu-utils-blueprint branch from bed42ae to 103a139 Compare April 11, 2018 20:49
@daibhin daibhin changed the title [WIP][Feature] util blueprint for module unification [Feature] util blueprint for module unification Apr 12, 2018
@GavinJoyce
Copy link
Member

@daibhin looking good, nice one!

Can you also add some tests to util-test-test.js? The test files should be colocated with the util function file, much like service-test in #16397

@daibhin daibhin force-pushed the dn/mu-utils-blueprint branch from 103a139 to e07054b Compare April 16, 2018 18:48
@daibhin
Copy link
Contributor Author

daibhin commented Apr 16, 2018

@GavinJoyce Updated the util-test-test.js file, let me know if there's anything else that needs to be done 😄

@GavinJoyce
Copy link
Member

GavinJoyce commented Apr 18, 2018

yarn run lint gives the follow error which is causing CI to fail:

screen shot 2018-04-18 at 08 57 27

running yarn run lint:fix might fix it

@daibhin daibhin force-pushed the dn/mu-utils-blueprint branch 2 times, most recently from be2156a to 1293e2b Compare April 18, 2018 20:07
@daibhin
Copy link
Contributor Author

daibhin commented Apr 18, 2018

Sorry about that, fixed! I'm still trying to figure out which is more amazing yarn run lint:fix or describe.only

@GavinJoyce
Copy link
Member

@daibhin as a final step, perhaps you could add an in-repo-addon test similar to this? This will ensure that we're not breaking existing functionality

@GavinJoyce
Copy link
Member

nice one @daibhin, this looks great

👍

@mixonic
Copy link
Member

mixonic commented Apr 27, 2018

Looks great! Thanks @daibhin :-D

@mixonic mixonic merged commit 979c4f5 into emberjs:master Apr 27, 2018
@daibhin daibhin deleted the dn/mu-utils-blueprint branch April 29, 2018 16:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants