Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Filter trees to only those containing global-usings or attributes prior to analyzing them. #62444
Filter trees to only those containing global-usings or attributes prior to analyzing them. #62444
Changes from 6 commits
6327162
2d58dd0
6f7fd29
7ccc099
e1ac870
9eeea23
9c97186
f61b83b
1705bd5
5999910
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
Large diffs are not rendered by 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.
@chsienki i need help here. for some reason the incremental generator is not tracking any steps here when the list filters down to the empty set. Can you help me debug through this to find out why that is?
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 looked at this, and I see whats going on. I think I have a fix for it, but its a little involved. I think we should open an issue here, and take the fix as a separate PR.
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.
Will do.
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.
Tracked with #62468
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.
Not part of your change, but wondering why we have both interface and abstract class flavors of this? Looks like the abstract class doesn't really add functionality here?
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.
So this is a pattern i follow when doing thigns with C# and VB. If you don't do this, then the VB side is more obnoxious as every impl has to include the interface information. the abstract approach means that both C# and VB just need to override.
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.
One developer's awesome feature that makes it clear when a method implements an interface method is another developer's annoying requirement to make me restate things that can be implicit ;)