- 
                Notifications
    
You must be signed in to change notification settings  - Fork 4.2k
 
Add project capabilities to C# Dev Kit project loading logs #80884
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
Conversation
…ate logging Co-authored-by: peterwald <[email protected]>
…check Co-authored-by: peterwald <[email protected]>
        
          
                ...erver/Microsoft.CodeAnalysis.LanguageServer.UnitTests/WorkspaceProjectFactoryServiceTests.cs
              
                Outdated
          
            Show resolved
            Hide resolved
        
      Co-authored-by: peterwald <[email protected]>
        
          
                ...erver/Microsoft.CodeAnalysis.LanguageServer.UnitTests/WorkspaceProjectFactoryServiceTests.cs
              
                Outdated
          
            Show resolved
            Hide resolved
        
      …urpose Co-authored-by: peterwald <[email protected]>
| [property: DataMember(Order = 2)] string? FilePath, | ||
| [property: DataMember(Order = 3)] IReadOnlyDictionary<string, string> BuildSystemProperties); | ||
| [property: DataMember(Order = 3)] IReadOnlyDictionary<string, string> BuildSystemProperties, | ||
| [property: DataMember(Order = 4)] ImmutableArray<string> ProjectCapabilities = default); | 
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.
Is the expectation the project capabilities are going to be used for something else down the road, or is this only for logging?
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.
(maybe my real question is "the PR description tells me what this does, it doesn't tell me why")
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.
The Test Explorer uses project capabilities, so we wanted to have logging that shows what their values are when loaded.
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.
The Test Explorer uses project capabilities, so we wanted to have logging that shows what their values are when loaded.
How is the C# extension involved here though - we don't utilize project capabilities? Wouldn't it be more appropriate to log on the CDK or test explorer side where they are produced or consumed? Imagine if there was some bug where the capabilities got sent to us, but not to the test explorer.
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.
That's a fair point. The Test Explorer already does log this info, but since our project query filters projects based on capability we don't know for sure what is happening when a test project doesn't get reported to us. The intent was to log this info further up the line to get more visibility. I saw this log entry in the logs, so that is what I targeted to add additional info. I guess we could target CDK instead.
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.
I guess we could target CDK instead.
That would be my preference - similar to the test explorer we're just a consumer of whatever CDK sends us. Logging on the CDK side seems more beneficial as they are the producers of this information. Additionally - I am not sure if anything will get logged here if the project changes after creation - this just logs the initial load.
| 
           @copilot Fix the formatting issues reported in the following build errors.  | 
    
Co-authored-by: peterwald <[email protected]>
Summary
Updated the logging for C# Dev Kit project loading to include project capabilities when available.
Changes Made
ProjectCapabilitiesfield toWorkspaceProjectCreationInfo(backward compatible with default value)WorkspaceProjectFactoryServiceto include capabilitiesLogProjectLoadWithCapabilitiesto better reflect its purposeTest Updates
The
LogProjectLoadWithCapabilitiestest:CapturingLoggerProviderto capture log messagesExpected Log Output
With capabilities:
Without capabilities (backward compatible):
Original prompt
💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.