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

Remove unnecessary lambda in diag service #77089

Merged
merged 3 commits into from
Feb 6, 2025

Conversation

CyrusNajmabadi
Copy link
Member

No description provided.

@CyrusNajmabadi CyrusNajmabadi requested a review from a team as a code owner February 6, 2025 21:01
@dotnet-issue-labeler dotnet-issue-labeler bot added Area-IDE untriaged Issues and PRs which have not yet been triaged by a lead labels Feb 6, 2025
project,
// Ensure we compute and return diagnostics for both the normal docs and the additional docs in this
// project if no specific document id was requested.
this.DocumentId != null ? [this.DocumentId] : [.. project.DocumentIds, .. project.AdditionalDocumentIds],
Copy link
Member

Choose a reason for hiding this comment

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

rather than having the documentId be nullable - would it be better if the API was modified to take in a set of documentIds as a required parameter? If we're updating the API to remove the getter anyway

Copy link
Member Author

Choose a reason for hiding this comment

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

maybe. i need to fully internalize all callers. If it is really "give me the diags for a project" and "give me diags for a single doc", then yes i think the final internal api would be as you mention, with two public entrypoints. I'm still just trying to make small incremental changes to get tehre :)

right now though, i don't mind the apis looking like ProjectId projectId, DocumentId? documentId to represent both those cases through one api.

@CyrusNajmabadi CyrusNajmabadi merged commit ff7628d into dotnet:main Feb 6, 2025
25 checks passed
@dotnet-policy-service dotnet-policy-service bot added this to the Next milestone Feb 6, 2025
@CyrusNajmabadi CyrusNajmabadi deleted the diagGetDocumentIds branch February 6, 2025 22:30
@akhera99 akhera99 modified the milestones: Next, 17.14 P2 Feb 25, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-IDE untriaged Issues and PRs which have not yet been triaged by a lead VSCode
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants