-
Notifications
You must be signed in to change notification settings - Fork 821
fix: Remove empty RestoreBindings and ClearBindings #13046 #21554
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
base: master
Are you sure you want to change the base?
fix: Remove empty RestoreBindings and ClearBindings #13046 #21554
Conversation
|
🤖 Your Docs stage site is ready! Visit it here: https://unodocsprstaging.z13.web.core.windows.net/pr-21554/docs/index.html |
src/SourceGenerators/Uno.UI.SourceGenerators/DependencyObject/DependencyObjectGenerator.cs
Outdated
Show resolved
Hide resolved
|
|
|
|
Xiaoy312
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you would need to update the last commit title to conform to conventional commits
ideally, you should rebase and squash them all into a single commit
....UI.SourceGenerators.Tests/DependencyObjectGeneratorTests/Given_DependencyObjectGenerator.cs
Outdated
Show resolved
Hide resolved
ee62d5f to
8f0ab14
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR removes empty RestoreBindings() and ClearBindings() methods that were previously kept for binary compatibility but contained no implementation. The changes clean up unused code by removing these obsolete methods from both the DependencyObjectStore.Binder class and the generated code in DependencyObjectGenerator.
Key changes:
- Removed empty
RestoreBindings()andClearBindings()methods fromDependencyObjectStore.Binder - Updated code generator to stop generating obsolete binding methods
- Added package diff ignore entries for the removed methods to maintain compatibility tracking
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
src/Uno.UI/UI/Xaml/DependencyObjectStore.Binder.cs |
Removed empty RestoreBindings() and ClearBindings() methods with their documentation |
src/SourceGenerators/Uno.UI.SourceGenerators/DependencyObject/DependencyObjectGenerator.cs |
Removed code generation for obsolete ClearBindings() and RestoreBindings() methods |
src/Uno.UI/UI/Xaml/IFrameworkElementImplementation.UIKit.tt |
Removed call to ClearBindings() method |
src/SourceGenerators/Uno.UI.SourceGenerators.Tests/DependencyObjectGeneratorTests/Given_DependencyObjectGenerator.cs |
Updated test expectations to reflect removal of obsolete methods and cleaned up formatting |
build/PackageDiffIgnore.xml |
Added ignore entries for removed methods to track API changes for version 6.2 |
|
🤖 Your WebAssembly Skia Sample App stage site is ready! Visit it here: https://unowasmprstaging.z20.web.core.windows.net/pr-21554/wasm-skia-net9/index.html |
|
🤖 Your Docs stage site is ready! Visit it here: https://unodocsprstaging.z13.web.core.windows.net/pr-21554/docs/index.html |
build/PackageDiffIgnore.xml
Outdated
| <Member fullName="System.Void Microsoft.UI.Xaml.Controls.ElementFactory.ClearBindings()" reason="Removed in Uno 6.2" /> | ||
| <Member fullName="System.Void Microsoft.UI.Xaml.Controls.ElementFactory.RestoreBindings()" reason="Removed in Uno 6.2" /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
need a regex here, since the methods are code-generated on multiple classes
| <Member fullName="System.Void Microsoft.UI.Xaml.Controls.ElementFactory.ClearBindings()" reason="Removed in Uno 6.2" /> | |
| <Member fullName="System.Void Microsoft.UI.Xaml.Controls.ElementFactory.RestoreBindings()" reason="Removed in Uno 6.2" /> | |
| <Member fullName="System\.Void .+\.(Clear|Restore)Bindings()" reason="Removed in Uno 6.2" /> |
build/PackageDiffIgnore.xml
Outdated
| <Member fullName="System.Void Microsoft.UI.Xaml.Controls.ElementFactory.add_DataContextChanged(Windows.Foundation.TypedEventHandler`2<Microsoft.UI.Xaml.FrameworkElement,Microsoft.UI.Xaml.DataContextChangedEventArgs> value)" reason="Not present in WinAppSDK 1.2" /> | ||
| <Member fullName="System.Void Microsoft.UI.Xaml.Controls.ElementFactory.remove_DataContextChanged(Windows.Foundation.TypedEventHandler`2<Microsoft.UI.Xaml.FrameworkElement,Microsoft.UI.Xaml.DataContextChangedEventArgs> value)" reason="Not present in WinAppSDK 1.2" /> | ||
| <Member fullName="System.Void Microsoft.UI.Xaml.Controls.ElementFactory.ClearBindings()" reason="Not present in WinAppSDK 1.2" /> | ||
| <Member fullName="System.Void Microsoft.UI.Xaml.Controls.ElementFactory.ClearBindings()" reason="Not present in WinAppSDK 1.2" /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
avoid whitespace changes if possible
| <Member fullName="System.Void Microsoft.UI.Xaml.Controls.ElementFactory.ClearBindings()" reason="Not present in WinAppSDK 1.2" /> | |
| <Member fullName="System.Void Microsoft.UI.Xaml.Controls.ElementFactory.ClearBindings()" reason="Not present in WinAppSDK 1.2" /> |
remember to rebase squash after, otherwise it is still gonna stain the history
build/PackageDiffIgnore.xml
Outdated
| </IgnoreSet> | ||
| <IgnoreSet baseVersion="6.1"> | ||
| <Types> | ||
| <Types> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| <Types> | |
| <Types> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
https://dev.azure.com/uno-platform/Uno%20Platform/_build/results?buildId=177761&view=logs&j=f63c6d40-e6cc-5158-041c-5ecaeb969bfb&t=dd05ac6d-5f8e-575b-7053-261df9674ec3&s=1ca05b26-5479-52b6-b481-8cd3b39f1708
test arent passing
just remove only of Clear/ResumeBindings and their attributes/comments, and keep everything else the same, whitespace included
| { | ||
|
|
||
| } | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
might as well, get rid of this extra empty line
8f0ab14 to
fad9620
Compare
|
🤖 Your Docs stage site is ready! Visit it here: https://unodocsprstaging.z13.web.core.windows.net/pr-21554/docs/index.html |
|
🤖 Your WebAssembly Skia Sample App stage site is ready! Visit it here: https://unowasmprstaging.z20.web.core.windows.net/pr-21554/wasm-skia-net9/index.html |
This PR removes empty RestoreBindings and ClearBindings methods from DependencyObjectStore.Binder
and adjusts the generated code in DependencyObjectGenerator.
Addresses #13046