-
Notifications
You must be signed in to change notification settings - Fork 392
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
Use new GetProjectDirectory method to reduce string allocations #9603
Conversation
/azp run |
Azure Pipelines successfully started running 1 pipeline(s). |
/azp run |
Azure Pipelines successfully started running 1 pipeline(s). |
/azp run |
Azure Pipelines successfully started running 1 pipeline(s). |
That insertion has now completed. |
a9397bf
to
10f917a
Compare
9063c6a
to
12377af
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice change!
@@ -894,7 +894,7 @@ internal async Task<string> GetLaunchSettingsFilePathNoCacheAsync() | |||
// see: https://github.com/dotnet/project-system/issues/2316. | |||
|
|||
// Default to the project directory if we're not able to get the AppDesigner folder. | |||
string folder = Path.GetDirectoryName(_commonProjectServices.Project.FullPath); | |||
string folder = _commonProjectServices.Project.GetProjectDirectory(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does the new GetProjectDirectory()
from CPS works cross-platform?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, it's in the host-agnostic layer so is available outside of VS too.
The CPS PRs were:
Avoids calling
Path.GetDirectoryName
in a bunch of places, which will allocate a new string for each call. The new extension method in CPS provides a singleton string for the project.Needs https://dev.azure.com/devdiv/DevDiv/_git/VS/pullrequest/595076 or newer to insert before being merged.
Microsoft Reviewers: Open in CodeFlow