Skip to content

Resolve addon file paths correctly in CLI >= 2.12#127

Merged
RobbieTheWagner merged 1 commit intoember-cli-code-coverage:masterfrom
dfreeman:fix-addon-paths
Oct 1, 2017
Merged

Resolve addon file paths correctly in CLI >= 2.12#127
RobbieTheWagner merged 1 commit intoember-cli-code-coverage:masterfrom
dfreeman:fix-addon-paths

Conversation

@dfreeman
Copy link
Contributor

Starting in (I believe) ember-cli@2.12, addon files stopped being nested under modules/, which means the path correction logic here wasn't working properly for them.

If you have thoughts on the best way to test this change, I'd welcome them 🙂

@RobbieTheWagner
Copy link
Collaborator

@dfreeman could we use ember-try and run coverage reports and check that things are renamed as expected? Aside from that, we could probably just do a quick test that passing the different file names yields the expected results here. Would definitely like at least a test that ensures the regex works as intended, if not some sort of full test.

@dfreeman
Copy link
Contributor Author

Unfortunately ember-try doesn't support swapping out versions of the CLI itself, but I can definitely add unit test coverage at least — I think I missed that this repo had a node-land test suite when I originally wrote this.

@RobbieTheWagner
Copy link
Collaborator

@dfreeman yeah, any sort of node tests to just lightly check this you can add would be fantastic, and then I will gladly merge 😄

Copy link
Collaborator

@rwjblue rwjblue left a comment

Choose a reason for hiding this comment

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

This looks good to me. Ironically, @dfreeman and I spent a bit of time chatting about the modules/ dir removal a little while back...

@RobbieTheWagner
Copy link
Collaborator

With approval from @rwjblue and since he told me he is planning a large refactor of this repo anyway, I will merge without tests. Thanks for the PR @dfreeman!

@RobbieTheWagner RobbieTheWagner merged commit b712f85 into ember-cli-code-coverage:master Oct 1, 2017
@dfreeman dfreeman deleted the fix-addon-paths branch October 2, 2017 14:11
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

Comments