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.

Sign up for GitHub

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

Jump to bottom

Warning or error is raised when consuming IL Event with an Obsolete attribute #14006

Merged
merged 5 commits into from
Oct 4, 2022
Merged

Warning or error is raised when consuming IL Event with an Obsolete attribute #14006

merged 5 commits into from
Oct 4, 2022

Conversation

edgarfgp
Copy link
Contributor

@edgarfgp edgarfgp commented Oct 1, 2022 •
edited
Loading

This PR fixes a BUG mentioned on #13511 (comment)

@edgarfgp edgarfgp marked this pull request as ready for review October 1, 2022 10:37
@edgarfgp edgarfgp changed the title No warning or error is raised when consuming IL Event with an Obsolete attribute Warning or error is raised when consuming IL Event with an Obsolete attribute Oct 2, 2022
vzarytovskii
vzarytovskii requested changes Oct 4, 2022
View reviewed changes
Copy link
Member

@vzarytovskii vzarytovskii left a comment

Choose a reason for hiding this comment

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

Just small comment on matching.
As a rule of thumb, we probably shouldn't ever match on any DUs from infos.

@T-Gro
Copy link
Member

T-Gro commented Oct 4, 2022

Just small comment on matching. As a rule of thumb, we probably shouldn't ever match on any DUs from infos.

I am planning more encapsulation around infos, Don lure me into this and I think it makes sense.
Reducing the amount of matching on Type/Method-Info, as well as amount of publicly exposed props from it.

@vzarytovskii vzarytovskii merged commit b68720d into dotnet:main Oct 4, 2022
@allisonchou allisonchou mentioned this pull request Oct 12, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@T-Gro T-Gro T-Gro approved these changes

@vzarytovskii vzarytovskii vzarytovskii approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@edgarfgp @T-Gro @vzarytovskii