Skip to content

Allow / in relative paths for using module (#7424) #12492

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
Merged
TravisEz13 merged 7 commits into from
Apr 30, 2020
Merged

Allow / in relative paths for using module (#7424) #12492

TravisEz13 merged 7 commits into from
Apr 30, 2020

Conversation

@jcotton42
Copy link
Contributor

@jcotton42 jcotton42 commented Apr 25, 2020 •
edited by iSazonov
Loading

PR Summary

Per the title, this makes using module work with a relative path that uses forward slashes. See the issue this is closing for more details.

Fix #7424.

PR Checklist

@jcotton42 jcotton42 requested a review from daxian-dbw as a code owner April 25, 2020 22:09
@ghost ghost assigned TravisEz13 Apr 25, 2020
@msftclas
Copy link

msftclas commented Apr 25, 2020 •
edited
Loading

CLA assistant check
All CLA requirements met.

@jcotton42 jcotton42 changed the title Close #7424. Allow / in relative paths for using module Allow / in relative paths for using module (#7424) Apr 25, 2020
@vexx32
Copy link
Collaborator

vexx32 commented Apr 26, 2020

@PoshChan please retry macos

@PoshChan
Copy link
Collaborator

PoshChan commented Apr 26, 2020

@vexx32, successfully started retry of PowerShell-CI-macOS

vexx32
vexx32 approved these changes Apr 26, 2020
View reviewed changes
Copy link
Collaborator

@vexx32 vexx32 left a comment

Choose a reason for hiding this comment

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

Looks good to me, nice work! 😊 💖

iSazonov
iSazonov reviewed Apr 26, 2020
View reviewed changes
@iSazonov iSazonov added the CL-General Indicates that a PR should be marked as a general cmdlet change in the Change Log label Apr 26, 2020
@iSazonov iSazonov added this to the 7.1.0-preview.3 milestone Apr 26, 2020
@jcotton42
Copy link
Contributor Author

jcotton42 commented Apr 26, 2020

Moved the test to the right file and updated it to use Pester's TestDrive

@jcotton42
Copy link
Contributor Author

jcotton42 commented Apr 26, 2020

CI for all 3 OSes failed, but it's code that, as far as I can tell, was unaffected by my change. And the tests ran just fine on the last commit, and the only change between the last commit and the current one is I moved my test and updated it.

@vexx32
Copy link
Collaborator

vexx32 commented Apr 26, 2020

@PoshChan please retry all

@PoshChan
Copy link
Collaborator

PoshChan commented Apr 26, 2020

@vexx32, successfully started retry of PowerShell-CI-static-analysis, PowerShell-CI-Windows, PowerShell-CI-macOS, PowerShell-CI-Linux

iSazonov
iSazonov reviewed Apr 26, 2020
View reviewed changes
@jcotton42
Resolve feedback
e98bc31 
Contains char instead of a string, and changing how the test checked for
success
TravisEz13
TravisEz13 reviewed Apr 27, 2020
View reviewed changes
Copy link
Member

@TravisEz13 TravisEz13 left a comment •
edited
Loading

Choose a reason for hiding this comment

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

requesting adding coverage for regressions.
I think the new code was covered well, but reviewing existing cases there is a gap.

iSazonov
iSazonov reviewed Apr 28, 2020
View reviewed changes
TravisEz13
TravisEz13 reviewed Apr 28, 2020
View reviewed changes
Copy link
Member

@TravisEz13 TravisEz13 left a comment

Choose a reason for hiding this comment

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

look good, pending addressing @iSazonov 's comments

iSazonov
iSazonov reviewed Apr 29, 2020
View reviewed changes
@TravisEz13
Copy link
Member

TravisEz13 commented Apr 29, 2020

@PoshChan Please remind me in 4 hours

@PoshChan
Copy link
Collaborator

PoshChan commented Apr 29, 2020

@TravisEz13, this is the reminder you requested 4 hours ago

@jcotton42
Copy link
Contributor Author

jcotton42 commented Apr 29, 2020 •
edited
Loading

TestCases refactor complete, should be ready for merging once CI passes.

@jcotton42
Copy link
Contributor Author

jcotton42 commented Apr 30, 2020

@PoshChan please retry ssh

@PoshChan
Copy link
Collaborator

PoshChan commented Apr 30, 2020

@jcotton42, you are not authorized to request a rebuild

@vexx32
Copy link
Collaborator

vexx32 commented Apr 30, 2020

@PoshChan please retry ssh

@PoshChan
Copy link
Collaborator

PoshChan commented Apr 30, 2020

@vexx32, I do not understand the build target(s) ssh; I only allow static, windows, macos, linux, all

@vexx32
Copy link
Collaborator

vexx32 commented Apr 30, 2020

@PoshChan please retry all

@PoshChan
Copy link
Collaborator

PoshChan commented Apr 30, 2020

@vexx32, successfully started retry of PowerShell-CI-static-analysis, PowerShell-CI-Windows, PowerShell-CI-macOS, PowerShell-CI-Linux

@vexx32 vexx32 mentioned this pull request Apr 30, 2020
Merged
14 tasks
iSazonov
iSazonov reviewed Apr 30, 2020
View reviewed changes
@jcotton42 @iSazonov
Update TestCases formatting
e9d51cb 
Co-authored-by: Ilya <darpa@yandex.ru>
@TravisEz13
Copy link
Member

TravisEz13 commented Apr 30, 2020

@PoshChan Please remind me in 1 hour

@PoshChan
Copy link
Collaborator

PoshChan commented Apr 30, 2020

@TravisEz13, this is the reminder you requested 1 hour ago

@TravisEz13 TravisEz13 changed the title Allow / in relative paths for using module (#7424) Allow / in relative paths for using module (#7424) Apr 30, 2020
@TravisEz13 TravisEz13 merged commit c19a417 into PowerShell:master Apr 30, 2020
@iSazonov
Copy link
Collaborator

iSazonov commented Apr 30, 2020

@jcotton42 Thanks for your contribution!

@ghost
Copy link

ghost commented May 19, 2020

🎉v7.1.0-preview.3 has been released which incorporates this pull request.:tada:

Handy links:

@ghost ghost mentioned this pull request May 19, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@TravisEz13 TravisEz13 TravisEz13 approved these changes

@iSazonov iSazonov iSazonov approved these changes

@daxian-dbw daxian-dbw Awaiting requested review from daxian-dbw daxian-dbw is a code owner

+1 more reviewer

@vexx32 vexx32 vexx32 approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@TravisEz13 TravisEz13

Labels

CL-General Indicates that a PR should be marked as a general cmdlet change in the Change Log

Projects

None yet

Milestone

7.1.0-preview.3

Development

Successfully merging this pull request may close these issues.

using module statement with a relative path not treated as relative to the location of the enclosing script when forward slashes are used

6 participants

@jcotton42 @msftclas @vexx32 @PoshChan @TravisEz13 @iSazonov