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.

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

Make ProjectNames.FromDTEProjectAsync more reliable #3776

Merged
merged 1 commit into from
Dec 2, 2020

Conversation

zivkan
Copy link
Member

@zivkan zivkan commented Nov 23, 2020

Bug

Fixes: https://github.com/NuGet/Client.Engineering/issues/602
Regression: Yes

  • Last working version: 5.6
  • How are we preventing it in future: Incrementally make ProjectNames.FromDTEProjectAsync more reliable

Fix

Details: EnvDTE.Project is implemented by project systems, not a generic implemented by the VS platform itself. Therefore, some project systems might have incomplete or buggy implementations. Try multiple ways to get the project GUID.

Testing/Validation

Tests Added: No
Reason for not adding tests: Problems only occurs on custom & 3rd party project systems that don't fully implement EnvDTE.Project
Validation: Need to insert into VS and monitor telemetry.

@zivkan zivkan requested a review from a team as a code owner November 23, 2020 03:12
@zivkan zivkan force-pushed the dev-zivkan-projectnames-from-dte-reliability branch from 8722c5c to 8cf4d7b Compare November 26, 2020 15:20
Copy link
Member

@nkolev92 nkolev92 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, one quick question.

}
}

projectGuid = vsSolution5.GetGuidOfProjectFile(fullName);
Copy link
Member

Choose a reason for hiding this comment

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

This probably needs moved to the top method instead right?

Copy link
Member Author

Choose a reason for hiding this comment

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

"unique name" is usually just the relative path to the project file from the solution. Every test I've tried so far returns the exact string that's in the project file. I feel like that's probably more likely to be reliable, hence I preferred it over DTE.FullName. But which of the two is tried first doesn't really matter to me. I can change it if you prefer.

Copy link
Member

Choose a reason for hiding this comment

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

All good as long as it's consistent.

}
}

projectGuid = vsSolution5.GetGuidOfProjectFile(fullName);
Copy link
Member

Choose a reason for hiding this comment

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

All good as long as it's consistent.

@zivkan zivkan merged commit 20f5701 into dev Dec 2, 2020
@zivkan zivkan deleted the dev-zivkan-projectnames-from-dte-reliability branch December 2, 2020 00:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants