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

TASK: Declare public api in php classes #3704

Merged
merged 2 commits into from
Feb 5, 2024

Conversation

mhsdesign
Copy link
Member

@mhsdesign mhsdesign commented Jan 30, 2024

Depends on #3682 which must be merged first.

I recently had someone ask me how to use this Property thing in Neos9. At first i was confused. And then i understood he was trying to use the Neos.Ui "change" classes.
To avoid that in the future and make it also easier to clean up our php mess in the Neos UI ^^ i placed a few internal annotations around.

and the changes now explicitly warn from being used :D (which is spoiler really hard anyway ^^ - otherwise we would have tests *g)

/**
 * Changes a property on a node
 * @internal These objects internally reflect possible operations made by the Neos.Ui.
 *           They are sorely an implementation detail. You should not use them!
 *           Please look into the php command API of the Neos CR instead.
 */
class Property extends AbstractChange

If you happen to use any class from the Neos Ui previously (which should not be the case, as the ui has nothing to offer for php users ^^), which is not declared as api anymore, you should please prepare to refactor it.

What I did

How I did it

How to verify it

NONE. Nearly. But what is this? A small village ...
@mhsdesign mhsdesign changed the base branch from 9.0 to feature/multi-app/01-foundation January 30, 2024 19:16
@mhsdesign mhsdesign changed the title TASK: Declare public api in php classes !!! TASK: Declare public api in php classes Jan 31, 2024
@mhsdesign mhsdesign changed the title !!! TASK: Declare public api in php classes TASK: Declare public api in php classes Jan 31, 2024
@mhsdesign mhsdesign deleted the branch neos:9.0 February 2, 2024 14:56
@mhsdesign mhsdesign closed this Feb 2, 2024
@mhsdesign mhsdesign reopened this Feb 2, 2024
@mhsdesign mhsdesign changed the base branch from feature/multi-app/01-foundation to 9.0 February 2, 2024 14:58
@@ -17,6 +17,7 @@
/**
* Contract for Node Creation handler that allow to hook into the process just before a node is being added
* via the Neos UI
* @api
Copy link
Member Author

Choose a reason for hiding this comment

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

to be transparent with you ;) that is the only php class/interface in the neos.ui which deserves an @api notation. Any objections? Or do you need another thing to be declared @api as well?

@@ -22,6 +22,7 @@

/**
* The Workspace helper for EEL contexts
* @todo EEL helpers are still to be declared as internal
Copy link
Member

Choose a reason for hiding this comment

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

And why you did not declare it ;)

Copy link
Member Author

Choose a reason for hiding this comment

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

because this would be to controversy to declare that in this pr. Will be discussed in a followup if the helper was actually useful in some obscure case.

Copy link
Member

@markusguenther markusguenther left a comment

Choose a reason for hiding this comment

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

Looks good to me, and thanks for going through all the files :)

@mhsdesign mhsdesign merged commit cb2b966 into neos:9.0 Feb 5, 2024
9 checks passed
@mhsdesign mhsdesign deleted the task/declarePublicApiInPhpClasses branch February 12, 2024 20:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants