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.

Sign up for GitHub

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

Jump to bottom

fix: use file.originalPath instead of file.path #218

Merged
merged 1 commit into from
Oct 5, 2017
Merged

fix: use file.originalPath instead of file.path #218

merged 1 commit into from
Oct 5, 2017

Conversation

tjenkinson
Copy link
Contributor

Because other plugins (such as https://github.com/gjurgens/karma-renamer-preprocessor) may change the path.

Please check if the PR fulfills these requirements

  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)

What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)

  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Other... Please describe:

What is the current behavior? (You can also link to an open issue here)
If this plugin is used with https://github.com/gjurgens/karma-renamer-preprocessor and singleRun is false, it fails on subsequent builds as it is looking at the renamed path, not the original one.

What is the new behavior?
If another plugin changes the path it won't effect how this works.

Does this PR introduce a breaking change?

  • Yes
  • No

@tjenkinson
Copy link
Contributor Author

I just signed the license would be great if this could be merged so we could point back to this instead of using our fork.

@tjenkinson
Copy link
Contributor Author

I'm not sure why it's still showing as unsigned. When I click details it shows as signed

@tjenkinson
Copy link
Contributor Author

hey this has been open for a while now. I was wondering if it could be merged?
ping @d3viant0ne @sokra !

@tjenkinson
Copy link
Contributor Author

@alexander-akait
Copy link
Collaborator

@tjenkinson can your add tests?

@michael-ciniawsky michael-ciniawsky added this to the 2.0.4 milestone Jun 27, 2017
@michael-ciniawsky michael-ciniawsky changed the title Use originalPath not path fix: use file.originalPath instead of file.path Jun 27, 2017
@tjenkinson
Copy link
Contributor Author

@evilebottnawi how do you suggest I test this? It's a 2 line change, and there aren't any other tests.

@joshwiens joshwiens closed this Oct 5, 2017
@tjenkinson
Copy link
Contributor Author

Hey @d3viant0ne why has this been closed it is fixing a bug isn't it? We are currently using a fork with this fix it would be nice to be able to use the main version.

@joshwiens
Copy link
Contributor

Closed the wrong PR

@joshwiens
Copy link
Contributor

Reopen it & target master

@tjenkinson
Copy link
Contributor Author

I don't have permission to reopen

@joshwiens joshwiens changed the base branch from v2 to master October 5, 2017 17:27
@joshwiens joshwiens reopened this Oct 5, 2017
@joshwiens
Copy link
Contributor

For reference, if you click edit by the title you can change the base branch ( that was preventing this from being reopened ).

@joshwiens
Copy link
Contributor

@tjenkinson - When you have time, rebase this with what is current master & we can get this wrapped up.

@tjenkinson
Copy link
Contributor Author

done!

@joshwiens joshwiens merged commit 0d075b0 into codymikol:master Oct 5, 2017
@michael-ciniawsky michael-ciniawsky removed this from the 2.0.7 milestone Nov 11, 2017
@dependencies dependencies bot mentioned this pull request Dec 5, 2017
Merged
mattlewis92 pushed a commit that referenced this pull request Dec 14, 2017
@tjenkinson @mattlewis92
fix: use file.originalPath instead of file.path (#218)
9520d6c 
Because other plugins (such as https://github.com/gjurgens/karma-renamer-preprocessor) may change the path.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@alexander-akait alexander-akait alexander-akait approved these changes

@michael-ciniawsky michael-ciniawsky michael-ciniawsky approved these changes

@joshwiens joshwiens joshwiens approved these changes

@bebraw bebraw Awaiting requested review from bebraw

Assignees
No one assigned
Labels
semver: Patch status: Approved type: Bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@tjenkinson @alexander-akait @joshwiens @michael-ciniawsky