Skip to content

Conversation

@lolmaus
Copy link
Contributor

@lolmaus lolmaus commented Nov 29, 2019

This might be not the right place to do the replacement, but it does the thing and unblocks my deploys.

@RobbieTheWagner
Copy link
Member

@lolmaus this seems to break the tests. Can you elaborate on why this is needed? Is it that windows generates paths with backslashes, then we need to convert them back? If would be great if we could get the existing stuff to always return the right paths, rather than manually changing them.

@lolmaus
Copy link
Contributor Author

lolmaus commented Nov 29, 2019

this seems to break the tests.

I don't think so. My branch is red because the master has been red when I branched off.

This seems to be the offending commit: https://travis-ci.org/ember-learn/ember-cli-addon-docs/builds/607226331 At least its commit message sounds related to the assertion error message.


Can you elaborate on why this is needed? Is it that windows generates paths with backslashes, then we need to convert them back?

Yes.


If would be great if we could get the existing stuff to always return the right paths, rather than manually changing them.

Well, yes, I generally agree, as I said in the top message. But I don't know where exactly the issue happens, and debugging Node is tedious.

My solution seems to be adequate and robust.

@RobbieTheWagner
Copy link
Member

@lolmaus can you rebase this please so we can see if the tests pass?

@lolmaus
Copy link
Contributor Author

lolmaus commented Dec 9, 2019

@rwwagner90 Passing now!

@RobbieTheWagner RobbieTheWagner merged commit b3df7dd into ember-learn:master Dec 9, 2019
@RobbieTheWagner
Copy link
Member

Thanks for the PR!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants