Skip to content

Conversation

@come-nc
Copy link
Collaborator

@come-nc come-nc commented Feb 3, 2025

Fix #17

Description

Add rules in 27 set to migrate all annotations to attributes.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

PR checklist

  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING.md document.
  • I have added tests to cover my changes.

@come-nc come-nc added the enhancement New feature or request label Feb 3, 2025
@come-nc come-nc self-assigned this Feb 3, 2025
come-nc added 2 commits March 10, 2025 14:23
Signed-off-by: Côme Chilliet <come.chilliet@nextcloud.com>
Signed-off-by: Côme Chilliet <come.chilliet@nextcloud.com>
@come-nc
Copy link
Collaborator Author

come-nc commented Mar 10, 2025

rectorphp/rector#9061

Either we wait for a fix or write our own Rector based on AnnotationToAttribute.
I think we may as well write our own as we will need special handling for AuthorizedAdminSetting anyway.

@provokateurin
Copy link
Contributor

And many other annotation/attribute pairs also work slightly different, so a custom solution is probably required.

come-nc added 2 commits March 11, 2025 16:30
This fixes migration for most annotations to attribute.
Commented out AuthorizedAdminSetting because it’s a more complex case.

Signed-off-by: Côme Chilliet <come.chilliet@nextcloud.com>
Signed-off-by: Côme Chilliet <come.chilliet@nextcloud.com>
@come-nc come-nc force-pushed the enh/add-annotations-migrations branch from de718b2 to 1dd2ef0 Compare March 11, 2025 15:41
Signed-off-by: Côme Chilliet <come.chilliet@nextcloud.com>
@come-nc come-nc marked this pull request as ready for review March 11, 2025 16:26
@come-nc
Copy link
Collaborator Author

come-nc commented Mar 11, 2025

Let’s merge as-is and note AuthorizedAdminSetting as a todo for later?

Copy link
Contributor

@provokateurin provokateurin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not too knowledgable about the internals of rector, but the result looks great 👍

@come-nc come-nc merged commit 869cc8d into main Mar 31, 2025
22 checks passed
@come-nc come-nc deleted the enh/add-annotations-migrations branch March 31, 2025 14:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Migrate annotations to attribute in 30 set

4 participants