-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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: resolve invalid conflict resolution #11077 for v6.5 #11356
Conversation
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.
This comes from a merge conflict that was incorrectly resolved. This issue was already resolved in #11076.
Take a look at the difference between:
The current PR is nice but you may rename it (and the commit too) to indicate that this is more a conflict resolution that a bug fix.
Something like:
fix: resolve invalid conflict resolution #11077 for v6.5
What do you think ?
Because this is wrong in 6.5.0 RC1. |
In general it is a bugfix. Your proposed commit message is not a clear and precise one as it does not describe the change imho. |
The commit title I proposed describe the purpose of the commit. We are not fixing a bug we just discovered in v6.5, we're re-applying a fix that was removed in a conflict resolution. The code change and the reasons for it are kept in the same place: the pull request we want to "re-import" #11077. |
Done. |
Nice. Thank you 👍 |
This would be more readable if lambdas and |
Description
Wrong local variable
_this
was used.Motivation and Context
Fix the wrong local variable, should be
this
ore.target
.Screenshots (if appropriate):
Types of changes
functionality to change)
Checklist (all required):
develop
orsupport/*
).