openzeppelin-upgrades
openzeppelin-upgrades copied to clipboard
Code for the rename catch is confusing.
This code is not really clear
const nameChange =
!this.unsafeAllowRenames &&
original.label !== updated.renamedFrom &&
(updated.label !== original.label ||
(updated.renamedFrom !== undefined && updated.renamedFrom !== original.renamedFrom));
(updated.renamedFrom !== undefined && updated.renamedFrom !== original.renamedFrom) is "if we are renaming, and we are not renaming what original renames from".
This is slower to run, but is IMO way clearer:
const nameChange =
!this.unsafeAllowRenames &&
(
// the original label is neither the new label, not the renamedFrom
![ updated.label, updated.renamedFrom ].includes(original.label) ||
// the renamedFrom is set, and doesn't match either the previous label, or the previous rename
![ undefined, original.label, original.renamedFrom ].includes(updated.renamedFrom)
)
The suggested code looks good to me. Can you open a PR?