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

AllMethInfosOfTypeInScope with shortcut of extension methods #4457

Merged
merged 2 commits into from
Apr 11, 2019

Conversation

forki
Copy link
Contributor

@forki forki commented Mar 7, 2018

When we check for method or property Infos we often don't need all Infos but only the first one. This pr avoids looking though all inheritance layers all the time.

@forki forki force-pushed the intrinsic branch 12 times, most recently from 6a92131 to 38cbe97 Compare March 7, 2018 09:37
@forki forki changed the title [WIP] AllMethInfosOfTypeInScope with shortcut of extension methods AllMethInfosOfTypeInScope with shortcut of extension methods Mar 7, 2018
@dsyme
Copy link
Contributor

dsyme commented Mar 7, 2018

@forki The code looks fine - but what's the reason for the change? e.g. write the description for the PR :) thanks!

@forki
Copy link
Contributor Author

forki commented Mar 7, 2018

Fone

@dsyme
Copy link
Contributor

dsyme commented Mar 9, 2018

@forki noch fone? :)

@forki
Copy link
Contributor Author

forki commented Mar 9, 2018

Fatfingered lol

@cartermp cartermp requested review from KevinRansom and dsyme and removed request for KevinRansom February 8, 2019 17:22
@cartermp
Copy link
Contributor

cartermp commented Feb 8, 2019

@forki looks like there are conflicts. Is this PR still good? Looks like it is on the surface at least

@forki
Copy link
Contributor Author

forki commented Feb 8, 2019 via email

@forki
Copy link
Contributor Author

forki commented Feb 10, 2019

rebased. let's see if it becomes green

@forki
Copy link
Contributor Author

forki commented Feb 11, 2019

@KevinRansom it looks like the build succeeded but some error during fcs publishing happened. ist this related?

@forki forki closed this Feb 11, 2019
@forki forki reopened this Feb 11, 2019
@cartermp
Copy link
Contributor

The FCS CI has some flakiness to it. As you've found, just close/re-open ought to solve it.

@cartermp
Copy link
Contributor

@dsyme you said the code looks fine, does that count as a sign-off?

@forki
Copy link
Contributor Author

forki commented Feb 11, 2019

would be good to double check the AtMostOnce calls. but I think I was very conservative about this here

@cartermp
Copy link
Contributor

Ugh I have half a mind to just delete that flaky test

@cartermp cartermp closed this Feb 13, 2019
@cartermp cartermp reopened this Feb 13, 2019
@dsyme
Copy link
Contributor

dsyme commented Feb 19, 2019

Why is NameResolution.fsi showing as 100% change in this PR?

@forki
Copy link
Contributor Author

forki commented Feb 19, 2019 via email

@forki
Copy link
Contributor Author

forki commented Feb 26, 2019

rebased again.

@forki forki force-pushed the intrinsic branch 2 times, most recently from 33d3888 to 984d5da Compare March 11, 2019 12:28
@forki
Copy link
Contributor Author

forki commented Mar 11, 2019

and rebased yet another time.
@KevinRansom I'd appreciate if we can either close it or get it in. Rebase/merges are a bit annoying right now with all that cleanup happening in those files.

@forki
Copy link
Contributor Author

forki commented Mar 12, 2019

and another time..

@KevinRansom
Copy link
Member

@forki,

thank you for this.

@KevinRansom KevinRansom merged commit 0c0dd3e into dotnet:master Apr 11, 2019
@cartermp cartermp added this to the 16.1 milestone Apr 11, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants