Skip to content

Conversation

@ericsnowcurrently
Copy link

@ericsnowcurrently ericsnowcurrently commented Aug 6, 2019

(for #6758)

The key changes:

  • leave node IDs alone (do not normcase)
  • preserve the "relpath" of each file/folder
  • send that relpath back in the results
  • generate "TestFile.fullPath" (in discovery parser) using relpath instead of ID

There is also a bunch of cleanup (in cases where the code was cluttered enough that I was worried about missing something). (sorry) If that is a problem then let me know and we can walk through the PR.

  • Pull request represents a single change (i.e. not fixing disparate/unrelated things in a single PR)
  • Title summarizes what is changing
  • [ ] Has a news entry file (remember to thank yourself!)
  • Appropriate comments and documentation strings in the code
  • [ ] Has sufficient logging.
  • [ ] Has telemetry for enhancements.
  • Unit tests & system/integration tests are added/updated
  • [ ] Test plan is updated as appropriate
  • [ ] package-lock.json has been regenerated by running npm install (if dependencies have changed)
  • [ ] The wiki is updated with any design decisions/details.

@ericsnowcurrently ericsnowcurrently added the no-changelog No news entry required label Aug 6, 2019
@karrtikr karrtikr self-assigned this Aug 6, 2019
@ericsnowcurrently
Copy link
Author

FYI, I know what the problem is in Windows CI and will fix it soon.

Copy link

@karrtikr karrtikr left a comment

Choose a reason for hiding this comment

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

As far as the key changes go, this looks good to me.

@ericsnowcurrently ericsnowcurrently merged commit 931757b into microsoft:master Aug 8, 2019
@ericsnowcurrently ericsnowcurrently deleted the fix-6758-parent-locations branch August 8, 2019 22:05
ericsnowcurrently pushed a commit that referenced this pull request Aug 12, 2019
…ery. (#6944)

(for #6940)

This is essentially a follow-up to #6877 (for #6758).
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

no-changelog No news entry required

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants