Skip to content

Conversation

@Sergio0694
Copy link
Member

This PR removes the nullability attributes generator, which is a change long overdue. This generator was just there to keep the code simpler when generating transitive members (eg. from [INotifyPropertyChanged]), but in hindsight pushing the extra complexity onto final users was not the right choice, as it resulted in somewhat unintuitive behavior. This is because users would find themselves with a couple of random polyfills for [NotNull] and [NotNullIfNotNull], which would not be expected and not really documented either, and also it could potentially conflict with other generators doing that explicitly.

This PR removes the generator entirely, and just updates the code to strip those attributes during generation if not available. That is, if those attributes are not available, the code will simply be generated without using those attributes at all.

PR Checklist

  • Created a feature/dev branch in your fork (vs. submitting directly from a commit on main)
  • Based off latest main branch of toolkit
  • PR doesn't include merge commits (always rebase on top of our main, if needed)
  • Tested code with current supported SDKs
  • Tests for the changes have been added (for bug fixes / features) (if applicable)
  • Header has been added to all new source files (run build/UpdateHeaders.bat)
  • Contains NO breaking changes
  • Every new API (including internal ones) has full XML docs
  • Code follows all style conventions

@Sergio0694 Sergio0694 added introduce breaking changes 💥 This change would be a breaking change next preview ✈️ This changes will be available in the upcoming preview mvvm-toolkit 🧰 Issues/PRs for the MVVM Toolkit labels Dec 14, 2022
@Sergio0694 Sergio0694 force-pushed the dev/remove-nullability-attributes branch from 4851285 to 82908a0 Compare December 19, 2022 19:29
@Sergio0694 Sergio0694 force-pushed the dev/remove-nullability-attributes branch from 82908a0 to da68b2c Compare December 19, 2022 19:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

introduce breaking changes 💥 This change would be a breaking change mvvm-toolkit 🧰 Issues/PRs for the MVVM Toolkit next preview ✈️ This changes will be available in the upcoming preview

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants