Skip to content

Add working directory parameter to Start-Job #10324

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
adityapatwardhan merged 16 commits into from
Sep 11, 2019
Merged

Add working directory parameter to Start-Job #10324

adityapatwardhan merged 16 commits into from
Sep 11, 2019

Conversation

@davinci26
Copy link
Contributor

@davinci26 davinci26 commented Aug 8, 2019 •
edited
Loading

PR Summary and Changes

Solves the same problem as #10274 (for future reference you can read the discussion) with a different implementation

  1. Enables the workingDirectory parameter when PowerShell runs in Server mode
  2. Uses the workingDirectory PowerShell parameter to implement start-job workingDirectory

Awaiting your feedback and comments!

PR Context

Fix #4287

PR Checklist

@davinci26 davinci26 mentioned this pull request Aug 8, 2019
Closed
14 tasks
PaulHigin
PaulHigin suggested changes Aug 8, 2019
View reviewed changes
Copy link
Contributor

@PaulHigin PaulHigin left a comment

Choose a reason for hiding this comment

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

Overall looks good. Just a few comments.

@ghost ghost added the Waiting on Author The PR was reviewed and requires changes or comments from the author before being accept label Aug 8, 2019
@ghost ghost removed the Waiting on Author The PR was reviewed and requires changes or comments from the author before being accept label Aug 9, 2019
PaulHigin
PaulHigin approved these changes Aug 9, 2019
View reviewed changes
Copy link
Contributor

@PaulHigin PaulHigin left a comment

Choose a reason for hiding this comment

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

The changes look good to me. However, it appears the rebase didn't work correctly because I now see changes from other PRs here.

@davinci26
Copy link
Contributor Author

davinci26 commented Aug 9, 2019

I saw that as well, I am working on a fix as we speak

Sotiris Nanopoulos added 5 commits August 9, 2019 09:03
starting working directory for powershell running in server mode
bf5a3c4
changed implementation to use a command
bf1daec
start job interface
24fef53
added test for bad directories, improved the error message, reverted …
4ad26b5 
…breaking change in PowerShellProcessInstance by making parameter optional and renamed parameter to workingDirectory in OutOfProcServerMediator to make it consistent
removed empty line in test
e4dc1f3
@davinci26 davinci26 force-pushed the feat-startJobWorkingDirectory branch from d0dd555 to e4dc1f3 Compare August 9, 2019 16:04
@davinci26 davinci26 mentioned this pull request Aug 9, 2019
Closed
8 tasks
adityapatwardhan
adityapatwardhan requested changes Aug 9, 2019
View reviewed changes
@ghost ghost added the Waiting on Author The PR was reviewed and requires changes or comments from the author before being accept label Aug 9, 2019
improved checking for nullOrWhitespace string for WorkingDirectory an…
62a7b0c 
…d improved testing using testcases patterns & testDrive
@ghost ghost removed the Waiting on Author The PR was reviewed and requires changes or comments from the author before being accept label Aug 9, 2019
@davinci26
Copy link
Contributor Author

davinci26 commented Aug 13, 2019

@adityapatwardhan I think I have addressed all your feedback. Let me know what you think

@adityapatwardhan adityapatwardhan changed the title Feat start job working directory Add working directory parameter to Start-Job Aug 20, 2019
adityapatwardhan
adityapatwardhan requested changes Aug 20, 2019
View reviewed changes
@ghost ghost added the Waiting on Author The PR was reviewed and requires changes or comments from the author before being accept label Aug 20, 2019
@davinci26
Copy link
Contributor Author

davinci26 commented Aug 21, 2019 •
edited
Loading

I also created some gifs to demo it

Start Job Working Directory Mac OSX

start job working directory OSX preview

Start Job Working Directory Windows

start job working directory Windows preview

@ghost ghost removed the Waiting on Author The PR was reviewed and requires changes or comments from the author before being accept label Aug 21, 2019
@davinci26
Copy link
Contributor Author

davinci26 commented Aug 29, 2019

@adityapatwardhan lemme know what you think about the changes now :)

adityapatwardhan
adityapatwardhan requested changes Sep 5, 2019
View reviewed changes
Copy link
Member

@adityapatwardhan adityapatwardhan left a comment

Choose a reason for hiding this comment

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

Some more changes in the tests. Thanks for your patience.

@ghost ghost added the Waiting on Author The PR was reviewed and requires changes or comments from the author before being accept label Sep 5, 2019
@adityapatwardhan
Copy link
Member

adityapatwardhan commented Sep 5, 2019

Please also look at CodeFactor issues.

@davinci26
Copy link
Contributor Author

davinci26 commented Sep 6, 2019

@adityapatwardhan I will do my best to clean up the CodeFactor issues in the files that I have touched.

But there are some issues that are related to documentation of parameters in functions that I have not touched so I am not sure if I am the best person to write this documentation.

Would this be ok?

@ghost ghost removed the Waiting on Author The PR was reviewed and requires changes or comments from the author before being accept label Sep 6, 2019
@adityapatwardhan adityapatwardhan merged commit f69f30b into PowerShell:master Sep 11, 2019
@adityapatwardhan
Copy link
Member

adityapatwardhan commented Sep 11, 2019

@davinci26 Thank you for your contribution!

@adityapatwardhan adityapatwardhan mentioned this pull request Sep 11, 2019
Closed
@davinci26 davinci26 mentioned this pull request Sep 12, 2019
Merged
9 tasks
@iSazonov iSazonov added the CL-General Indicates that a PR should be marked as a general cmdlet change in the Change Log label Sep 12, 2019
@iSazonov iSazonov added this to the 7.0.0-preview.4 milestone Sep 12, 2019
TravisEz13 pushed a commit to TravisEz13/PowerShell that referenced this pull request Sep 14, 2019
@davinci26 @TravisEz13
Add working directory parameter to Start-Job (PowerShell#10324)
cb53eb6
@ghost
Copy link

ghost commented Sep 19, 2019

🎉v7.0.0-preview.4 has been released which incorporates this pull request.:tada:

Handy links:

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

Reviewers

@adityapatwardhan adityapatwardhan adityapatwardhan approved these changes

@anmenaga anmenaga Awaiting requested review from anmenaga

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

@TravisEz13 TravisEz13 Awaiting requested review from TravisEz13 TravisEz13 is a code owner

+1 more reviewer

@PaulHigin PaulHigin PaulHigin approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@adityapatwardhan adityapatwardhan

Labels

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

Projects

None yet

Milestone

7.0.0-preview.4

Development

Successfully merging this pull request may close these issues.

Start-Job needs a -WorkingDirectory parameter

4 participants

@davinci26 @adityapatwardhan @PaulHigin @iSazonov