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

Removes dead code from type conversion #19436

Merged
merged 1 commit into from
Feb 12, 2024

Conversation

vitek-karas
Copy link
Member

Description of Change

The removed code would only run if converter was not null. But in that case the if on line 166 would match and it would return from the method. So the removed code can only be reached if converter is null.

The removed code would only run if `converter` was not null. But in that case the if on line 166 would match and it would return from the method. So the removed code can only be reached if `converter` is null.
@vitek-karas vitek-karas requested a review from a team as a code owner December 15, 2023 10:38
@ghost ghost added the community ✨ Community Contribution label Dec 15, 2023
@ghost
Copy link

ghost commented Dec 15, 2023

Hey there @vitek-karas! Thank you so much for your PR! Someone from the team will get assigned to your PR shortly and we'll get it reviewed.

@PureWeen PureWeen requested review from StephaneDelcroix and removed request for jfversluis and mattleibow December 15, 2023 18:08
@PureWeen
Copy link
Member

/azp run

Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@samhouts samhouts added the stale Indicates a stale issue/pr and will be closed soon label Jan 8, 2024
@jsuarezruiz jsuarezruiz added area-xaml XAML, CSS, Triggers, Behaviors and removed stale Indicates a stale issue/pr and will be closed soon labels Jan 16, 2024
@mattleibow
Copy link
Member

I wonder if it is better to still remove this dead code, then rather wrap the try in an if instead of the type convert? Makes it more readable and removes an unnecesary cast.

@StephaneDelcroix
Copy link
Contributor

StephaneDelcroix commented Jan 19, 2024

I believe this code was there to handle System.ComponentModel.TypeConverter in addition of XF TypeConverter, but we a quick check indicates we got rid of that

@jsuarezruiz
Copy link
Contributor

/azp run

Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@rmarinho rmarinho merged commit 604c217 into dotnet:main Feb 12, 2024
49 of 52 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Mar 15, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants