-
-
Notifications
You must be signed in to change notification settings - Fork 479
Add command to transfer ownership #1609
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
Conversation
| $package->addMaintainer($maintainer); | ||
| } | ||
|
|
||
| $this->doctrine->getManager()->persist(AuditRecord::packageTransferred($package, null, $oldMaintainers, array_values($maintainers))); |
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.
What about packages that were already own by the new maintainers ? Should they actually be marked as transferred to the same maintainers ?
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.
Based on my understanding, this transfer command is only necessary when people no longer have access, and it's highly unlikely that current maintainers remain in place. Besides that, storing all old and new maintainers also tells us exactly what arguments the command was used with?
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.
When applying that to a whole vendor namespace, the existing packages might have different maintainers. Maybe one of them already has only the new maintainers.
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.
Yeah I think what we could do here is at least check that the old and new arent' an exact match and if they are skip transferring that package as that's a noop. I'll add that post merge.
|
Alright cool, works well :) https://packagist.org/audit-log?type%5B%5D=package_transferred |
Adds a
packagist:transfer-ownershipcommand to change maintainers for multiple packages at once.