Removed Deprecated for 7.0 #3435
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…onExtensions.Light, and some other smaller methods marked as Obsolete.
|
Thanks azchohfi for opening a Pull Request! The reviewers will test the PR and highlight if there is any conflict or changes required. If the PR is approved we will proceed to merge the pull request 🙌 |
ghost
requested a review
from 
|
azchohfi
changed the title
|
Ok, I've updated the initial message with everything that got deleted. |
|
Should we do #3396 here too or leave that as a separate PR? |
|
I can do that on a different PR. |
|
Or here... |
There was a problem hiding this comment.
We should open a docs PR for this as well, which moves the docs to the 'obsolete' section?
@Sergio0694 we should also have a migration guide for folks for NotifyTaskCompletion to the new MVVM pattern. Think you said you'd add that to the samples/docs repo somewhere?
Microsoft.Toolkit.Uwp.UI.Animations/Extensions/AnimationExtensions.Light.cs
Outdated
Show resolved
Hide resolved
…/AnimationExtensions.Light, and some other smaller methods marked as Obsolete." This reverts commit 274492b.
…r methods marked as Obsolete.
azchohfi
force-pushed
the
removeObsolete7
branch
from
3effba3 to
75da45c
Compare
This was referenced Aug 20, 2020
7 tasks
michael-hawker
approved these changes
Sep 10, 2020
| @@ -35,12 +35,11 @@ Once you do a search, you should see a list similar to the one below (versions m | |||
| | --- | --- | | |||
| | Microsoft.Toolkit | .NET Standard NuGet package containing common code | | |||
| | Microsoft.Toolkit.HighPerformance | .NET Standard and .NET Core NuGet package with performance oriented helpers, extensions, etc. | | |||
| | Microsoft.Toolkit.Parsers | .NET Standard NuGet package containing cross-platform parsers, such as Markdown and RSS | | |||
| | Microsoft.Toolkit.Services | .NET Standard NuGet package containing cross-platform services | | |||
| | Microsoft.Toolkit.Parsers | .NET Standard NuGet package containing cross-platform parsers, such as Markdown | | |||
There was a problem hiding this comment.
I know we haven't gotten there yet, but should we mark the current Markdown parser as Obsolete at least now?
If we don't migrate the UI component soon, we could internalize all of this code just for the control usage... right?
Rosuavio
reviewed
Sep 11, 2020
|
Good catch @RosarioPulella, yeah, unfortunately GitHub doesn't allow us to comment on things outside the change (would be nice!). For now, since this is a branch we own in the repo, you can just make the change yourself on the branch and push a commit. Otherwise, you could just ping the original submitter with the suggestion in your comment. |
or any other in this solution.
Rosuavio
force-pushed
the
removeObsolete7
branch
from
c1307aa to
f09c909
Compare
Rosuavio
approved these changes
Sep 11, 2020
|
Hello @RosarioPulella! Because this pull request has the p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (
|
ghost
pushed a commit
that referenced
this pull request
Oct 27, 2020
## Part of #3062, related to #3435 <!-- Add the relevant issue number after the "#" mentioned above (for ex: Fixes #1234) which will automatically close the issue once the PR is merged. --> <!-- Add a brief overview here of the feature/bug & fix. --> ## PR Type What kind of change does this PR introduce? <!-- Please uncomment one or more that apply to this PR. --> - Optimization - Deprecation <!-- - Bugfix --> <!-- - Feature --> <!-- - Code style update (formatting) --> <!-- - Refactoring (no functional changes, no api changes) --> <!-- - Build or CI related changes --> <!-- - Documentation content changes --> <!-- - Sample app changes --> <!-- - Other... Please describe: --> ## What is the current behavior? <!-- Please describe the current behavior that you are modifying, or link to a relevant issue. --> The `Microsoft.Toolkit` package has a number of extensions for 2D arrays that are inefficient (they're both a bit slow and causing unnecessary memory allocations) and replaced by equivalent APIs in the `Microsoft.Toolkit.HighPerformance` package. https://github.com/windows-toolkit/WindowsCommunityToolkit/blob/30452cf0bbf627f12825cf50bbd31b0e526b9abe/Microsoft.Toolkit/Extensions/ArrayExtensions.cs#L27 https://github.com/windows-toolkit/WindowsCommunityToolkit/blob/30452cf0bbf627f12825cf50bbd31b0e526b9abe/Microsoft.Toolkit/Extensions/ArrayExtensions.cs#L48 https://github.com/windows-toolkit/WindowsCommunityToolkit/blob/30452cf0bbf627f12825cf50bbd31b0e526b9abe/Microsoft.Toolkit/Extensions/ArrayExtensions.cs#L68 ## What is the new behavior? <!-- Describe how was this issue resolved or changed? --> This PR makes these APIs obsolete, and also includes a number of small optimizations to existing code that was added during the refactoring to remove dependencies on these APIs, so that it'll be possible to remove them in the future with no issues. ## PR Checklist Please check if your PR fulfills the following requirements: - [X] Tested code with current [supported SDKs](../readme.md#supported) - [ ] Pull Request has been submitted to the documentation repository [instructions](..\contributing.md#docs). Link: <!-- docs PR link --> - [ ] ~~Sample in sample app has been added / updated (for bug fixes / features)~~ - [ ] ~~Icon has been created (if new sample) following the [Thumbnail Style Guide and templates](https://github.com/windows-toolkit/WindowsCommunityToolkit-design-assets)~~ - [X] Tests for the changes have been added (for bug fixes / features) (if applicable) - [X] Header has been added to all new source files (run *build/UpdateHeaders.bat*) - [X] Contains **NO** breaking changes
This pull request was closed.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Removed:
This is still missing the update on the docs repo.
Contributes towards #3062
Fixes #3396
PR Type
What kind of change does this PR introduce?
What is the current behavior?
Obsolete classes marked as obsolete, but still exist in code.
What is the new behavior?
Some classes/methods marked as obsolete are now removed from code.
PR Checklist
Please check if your PR fulfills the following requirements:
This PR IS a breaking change, as instead of just providing a warning, any developers that uses the dead code will now get missing method/class exceptions.