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

Replace join-path usages with the path library #3041

Merged
merged 1 commit into from
Feb 20, 2020
Merged

Replace join-path usages with the path library #3041

merged 1 commit into from
Feb 20, 2020

Conversation

PVince81
Copy link
Contributor

Description

Since we now include the "path" library, we don't need the "join-path"
library which only provides a subset of its features.

Related Issue

Fixes #2922

Motivation and Context

How Has This Been Tested?

  • Manual test of relevant code pieces

Screenshots (if appropriate):

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Technical debt
  • Tests

Checklist:

  • Code changes
  • Unit tests added
  • Acceptance tests added
  • Documentation ticket raised:

Open tasks:

  • ...

@PVince81 PVince81 self-assigned this Feb 14, 2020
@update-docs
Copy link

update-docs bot commented Feb 14, 2020

Thanks for opening this pull request! The maintainers of this repository would appreciate it if you would create a changelog item based on your changes.

@PVince81 PVince81 mentioned this pull request Feb 14, 2020
Closed
@LukasHirt
Copy link
Contributor

Looking at the tests error it seems it might be still somewhere imported?

@PVince81
Copy link
Contributor Author

oh, so it seems we used this lib in some tests as well. I'll recheck...

@PVince81
Copy link
Contributor Author

it looks like join-path doesn't behave exactly like path.join when dealing with URLs, so I can't replace it in the tests as a lot of test code has such assumption in place.

I'll keep the join-paths dependency for now for tests

Replace join-path usages with the path library
4e2586c 
Since we now include the "path" library, we don't need the "join-path"
library which only provides a subset of its features.
@PVince81
Copy link
Contributor Author

pushed. let's see...

@PVince81
Copy link
Contributor Author

@LukasHirt it went through this time

@LukasHirt
Copy link
Contributor

I guess we don't need changelog item for this, right?

@PVince81 PVince81 merged commit 1c90c16 into master Feb 20, 2020
@delete-merged-branch delete-merged-branch bot deleted the replace-join-path-with-path branch February 20, 2020 09:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@LukasHirt LukasHirt LukasHirt approved these changes

@DeepDiver1975 DeepDiver1975 Awaiting requested review from DeepDiver1975

Assignees

@PVince81 PVince81

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Get rid of join-path lib and use path lib instead
2 participants
@PVince81 @LukasHirt