Skip to content

Add Start-EditorServices script from vscode-powershell repo #639

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.

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

Merged
merged 9 commits into from
Mar 22, 2018

Conversation

rjmholt
Copy link
Contributor

@rjmholt rjmholt commented Mar 21, 2018

Adds the Start-EditorServices.ps1 script into the actual PowerShellEditorServices repo, to make PSES self-contained

@TylerLeonhardt
Copy link
Member

@rkeithhill
Copy link
Contributor

Just make sure we keep the version with the recently added logging info. :-)

@TylerLeonhardt
Copy link
Member

@rkeithhill yeah that PR was in vscode-powershell :)

Copy link
Member

@TylerLeonhardt TylerLeonhardt left a comment

Choose a reason for hiding this comment

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

Thanks for this @rjmholt! LGTM

@TylerLeonhardt
Copy link
Member

There's 1 test failure I'm trying to figure out. Not sure if it's a bad test or not.

@TylerLeonhardt
Copy link
Member

wait... that's not running all the tests.

@TylerLeonhardt
Copy link
Member

I got them all to run with this:
https://ci.appveyor.com/project/PowerShell/powershelleditorservices/build/1.6.0.1110/tests but there are a lot of failures... but they're related

@@ -11,6 +11,7 @@
using System.Diagnostics;
using System.IO;
using System.Text;
Copy link
Contributor

@rkeithhill rkeithhill Mar 22, 2018

Choose a reason for hiding this comment

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

Can you add this right before this line:

#if CoreCLR
using System.Reflection;
#endif

Without this, my build within VS 2017 fails with:

Severity	Code	Description	Project	File	Line	Suppression State
Error	CS1061	'Type' does not contain a definition for 'GetTypeInfo' and no extension method 'GetTypeInfo' accepting a first argument of type 'Type' could be found (are you missing a using directive or an assembly reference?)	PowerShellEditorServices.Test.Host(netcoreapp2.0)	C:\Users\Keith\GitHub\rkeithhill\PowerShellEditorServices\test\PowerShellEditorServices.Test.Host\ServerTestsBase.cs	42	Active

TylerLeonhardt and others added 3 commits March 21, 2018 20:46
Change to check length of sessionPath file and only if file exists test passes.
Update to use CodeBase which is the original location of the file under bin dir.  Location is in the temp dir when run under xUnit.
@TylerLeonhardt
Copy link
Member

@rkeithhill IS THE BEST

Copy link
Member

@TylerLeonhardt TylerLeonhardt left a comment

Choose a reason for hiding this comment

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

LGTM

@rjmholt
Copy link
Contributor Author

rjmholt commented Mar 22, 2018

@rkeithhill Wait, so do we want System.Reflection but don't want to wrap it with #if CoreCLR?

@rjmholt rjmholt force-pushed the add-startpses-script branch from 235a4f6 to 3d8a322 Compare March 22, 2018 18:30
@rjmholt
Copy link
Contributor Author

rjmholt commented Mar 22, 2018

Oh, I see, @tylerl0706 already fixed it. Just got rid of that last commit

@rjmholt
Copy link
Contributor Author

rjmholt commented Mar 22, 2018

LGTM

@TylerLeonhardt
Copy link
Member

Alright, I'm going to merge this in since we currently use the Start-EditorServices.ps1 file so we know that works and the tests are all passing.

@TylerLeonhardt TylerLeonhardt merged commit 781f573 into PowerShell:master Mar 22, 2018
@rjmholt rjmholt deleted the add-startpses-script branch December 12, 2018 06:02
TylerLeonhardt pushed a commit to TylerLeonhardt/PowerShellEditorServices that referenced this pull request Feb 26, 2019
Simplified extension setting description strings
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants