-
Notifications
You must be signed in to change notification settings - Fork 4.1k
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
Conversation
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], |
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.
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
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. 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.
No description provided.