Skip to content

Conversation

@psfinaki
Copy link
Contributor

@psfinaki psfinaki commented Nov 24, 2022

Partially fixes #14382
Fixes: #14393

Before:

2022-11-24.16-54-11.mp4

After:

2022-11-24.16-57-40.mp4

I found this nearly unused properly which seems to do the trick.

Note this does not solve the issue completely:

2022-11-24.17-00-50.mp4

@psfinaki psfinaki requested a review from a team as a code owner November 24, 2022 16:01
@psfinaki
Copy link
Contributor Author

Thanks @kerams for the suggestions btw.

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.

If the property is part of public surface API, it's a breaking change, then we will either need to bump major FCS version, or leave old one, mark it as obsolete and suggest using new one .

Also, how does it work with SRTP, iwsams generic functions and methods? What does it display?

@psfinaki
Copy link
Contributor Author

If the property is part of public surface API, it's a breaking change, then we will either need to bump major FCS version, or leave old one, mark it as obsolete and suggest using new one.

Mhm I'm allergic to typos but alright let's revert the breaking change and then solve it separately

Also, how does it work with SRTP, iwsams generic functions and methods? What does it display?

Will check it and add some extra tests maybe, stay tuned.

@vzarytovskii
Copy link
Member

If the property is part of public surface API, it's a breaking change, then we will either need to bump major FCS version, or leave old one, mark it as obsolete and suggest using new one.

Mhm I'm allergic to typos but alright let's revert the breaking change and then solve it separately

Understandable, but we should start being more careful with breaking changes in minor versions, it's a big pain point for community tooling

@psfinaki psfinaki requested review from a team and 0101 November 28, 2022 17:07
@psfinaki
Copy link
Contributor Author

So @vzarytovskii IWSAM is working correctly (added a test), SRTP has a small issue, will deal with it separately here.

@psfinaki psfinaki enabled auto-merge (squash) November 29, 2022 10:35
@vzarytovskii vzarytovskii merged commit 71b6329 into dotnet:main Nov 29, 2022
@psfinaki psfinaki deleted the psfinaki/hints-5 branch November 29, 2022 11:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

Make Type Hints less intrusive Type Hints showing unsolved type in for expression

3 participants