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

Revert "Remove unused functions (#20029)" #20050

Merged
merged 1 commit into from
Jan 22, 2024

Conversation

drasticactions
Copy link
Contributor

This reverts commit b4bc1c8.

This code was used by XAML Hot Reload, which is injected into an MAUI Application after the fact. The code for it is not contained within this repo, so it's not represented as being "used," but it is very much part of the API we need.

Description of Change

Revert the commit to add back the needed methods.

Issues Fixed

Fixes #20049

@symbiogenesis
Copy link
Contributor

symbiogenesis commented Jan 22, 2024

In that case, the IndexOf function should ideally receive the same 4 lines of code as in PR #19963

It may help improve HotReload performance and memory usage.

@drasticactions
Copy link
Contributor Author

drasticactions commented Jan 22, 2024

In that case, the IndexOf function should ideally receive the same 4 lines of code as in PR #19963

It may help improve HotReload performance and memory usage.

Sure, I'm happy to switch it to a new method or whatever needs to happen. But the removed method was accessed in code currently shipping in Visual Studio (for several releases), and that PR broke a shipping product. That method should be labeled obsolete instead of removed, so it won't retroactively break Visual Studio.

Right now, I just want to make sure we don't break customers.

e. To be clear, I don't blame anyone for not knowing those extension methods were being used outside of this source repo, especially since they are in an Internal class (to which partner extensions like ours have access). This speaks to a broader problem of not having tests for dependencies like ours run inside of this repo as part of its test run, so that you could have saw the breakage (outside of running the MAUI source projects inside of Visual Studio with XAML Hot Reload enabled, which also would have shown the exception)

@@ -20,6 +20,29 @@ public static void ForEach<T>(this IEnumerable<T> enumeration, Action<T> action)
}
}

/// <summary>
///
Copy link
Contributor

Choose a reason for hiding this comment

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

Empty documentation

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is reverting the already existing code.

Copy link
Member

@mattleibow mattleibow left a comment

Choose a reason for hiding this comment

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

Is it possible to add some notes in the code that we should not delete this? An unused internal method is hard to not delete...

@mattleibow mattleibow enabled auto-merge (squash) January 22, 2024 14:21
@mattleibow mattleibow merged commit 4618282 into dotnet:main Jan 22, 2024
47 checks passed
@rmarinho
Copy link
Member

Yeah just to add that this is an Internal api and that's why removing didn't broke the PublicAPI . But I agree we should add a some comment saying why this is used and mark ir as Obsolete in a next PR, can we do that @drasticactions ?
We could also try design a better public api for this and obsolete this one on net9

@drasticactions
Copy link
Contributor Author

@rmarinho @mattleibow I've made a new PR to add the tags #20068

@drasticactions drasticactions deleted the dev/timill/revert-20029 branch January 22, 2024 14:36
@github-actions github-actions bot locked and limited conversation to collaborators Feb 22, 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.

[Nightly] Removed EnumerableExtensions break MAUI Xaml Hot Reload
6 participants