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: Remove un-necessary toString method in FusionPathProxy #1500

Merged
merged 2 commits into from Apr 28, 2017

Conversation

kitsunet
Copy link
Member

@kitsunet kitsunet commented Mar 29, 2017

This was needed at some point to evaluate the proxy to a string in a Fluid template,
but the way a proxy is handled was changed some time ago so that the respective
methods are called to get the content of the proxy instead of just string casting,
therefore it was no longer needed.

And the exception handling in the toString is not a good idea anyway (but necessary
because toString cannot raise exceptions) so all in all this method is undesirable and
as we don't use it anymore it should be removed.

This is basically the result of a long debugging session at the last sprint where we
implemented a short term bugfix and figured that we don't need this method anymore
and should remove it in one of the next releases.

@mention-bot
Copy link

@kitsunet, thanks for your PR! By analyzing the history of the files in this pull request, we identified @radmiraal, @johannessteu and @skurfuerst to be potential reviewers.

@gerhard-boden
Copy link
Contributor

gerhard-boden commented Apr 12, 2017

Resolved the conflict, looks good to me codewise (looked for usage, but could not find any).
Could you give me a short info how come we don't need that method anymore, or why we had thiscode in the first place?

@kdambekalns kdambekalns changed the title TASK: Remove un-necessary toString method in FusionPathProxy TASK: Remove un-necessary toString method in FusionPathProxy Apr 28, 2017
Copy link
Member

@kdambekalns kdambekalns left a comment

Choose a reason for hiding this comment

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

Please, can you give some info about why this is no longer needed?

@kitsunet
Copy link
Member Author

This was needed at some point to evaluate the proxy to a string in a fluid template, but the way a proxy is handled was changed some time ago so that the respective methods are called to get the content of the proxy instead of just string casting, therefore it was no longer needed. And the exception handling in the toString is not a good idea anyway (but necessary because toString cannot raise exceptions) so all in all this method is undesirable and as we don't use it anymore it should be removed.

This is basically the result of a long debugging session at the last sprint where we implemented a short term bugfix and figured that we don't need this method anymore and shoudl remove it in one of the next releases.

@kdambekalns ☝️

@kdambekalns kdambekalns merged commit f457e55 into neos:master Apr 28, 2017
@kdambekalns kdambekalns deleted the task/remove-unnecessary-tostring branch April 28, 2017 07:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants