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

Fix Unpatch default parameter #465

Merged
merged 1 commit into from
Mar 23, 2022
Merged

Conversation

Windows10CE
Copy link
Contributor

Unpatch eventually calls into Patch.Remove, where the logic checks for "*" as the "remove all" wildcard, not null, which is the current default of Unpatch.

@pardeike
Copy link
Owner

@Windows10CE
Copy link
Contributor Author

That article would count this change as a breaking one, yes, because the default behavior will change from doing nothing to unpatching all patches on a method. As I wouldn't expect a default optional parameter to have a method do nothing, the new default is likely what callers expected to happen.

So, there are two scenarios after a recompile:

  1. Callers now get the expected behavior and everything works fine (not changing this here would've required a bugfix at the caller level)
  2. Callers get expected behavior, but their code breaks as a result. However, this happening is basically a "I was relying on that broken code!".

Whether or not you consider it a breaking change depends on if you consider a bugfix that changes something to the expected behavior when people could've been relying (on accident) on the broken behavior to be a breaking change

@pardeike pardeike merged commit 58b3acd into pardeike:master Mar 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants