Skip to content
This repository has been archived by the owner on Mar 3, 2023. It is now read-only.

Relativize spec paths more when reporting #13825

Merged
merged 1 commit into from
Feb 23, 2017
Merged

Conversation

50Wliu
Copy link
Contributor

@50Wliu 50Wliu commented Feb 15, 2017

Requirements

  • Filling out the template is required. Any pull request that does not include enough information to be reviewed in a timely manner may be closed at the maintainers' discretion.
  • All new code requires tests to ensure against regressions

Description of the Change

First, here's a before/after comparison on Windows:

Before After
specs-before specs-after

Much nicer.

Changes include:

  • Updating to the new .<anonymous> syntax
  • Stripping away file:/// identifiers on Windows paths and converting the rest of the path to use Windows path separators (in order for it to get relativized correctly)
  • Relativizing paths in parentheses

Alternate Designs

Instead of relativizing the path twice, depending on if its in parentheses or not, a more general line.replace(///#{_.escapeRegExp(spec.specDirectory}#{_.escapeRegExp(path.sep}///, '') could be used. I am unsure if that would potentially relativize more than we wanted.

Why Should This Be In Core?

Because it fixes already-implemented code in core.

Benefits

See before/after picture above.

Possible Drawbacks

I don't foresee any.

Applicable Issues

None.

/cc @damieng for the Windows portions.

* `[object Object].anonymous` changed a while back to just `.anonymous`
* Windows uses `file:///path` notation, so take that into account
* Also relativize paths in parentheses
@50Wliu
Copy link
Contributor Author

50Wliu commented Feb 15, 2017

If specs are necessary for this change I can add them.

@damieng
Copy link
Contributor

damieng commented Feb 15, 2017

This looks good to me. I'm okay with keeping windows-style paths for local stuff although for the areas where we report remotely - bugsnag, GA, notification to github issue - I prefer to normalize them to Unix style so that they are correctly combined into a single issue where appropriate.

Thanks for this!

@50Wliu 50Wliu merged commit 82faad9 into master Feb 23, 2017
@50Wliu 50Wliu deleted the wl-relativize-specs-more branch February 23, 2017 22:44
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants