Skip to content

Removed use of 'Magento\Framework\Filesystem' #10671

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

Closed
wants to merge 1 commit into from
Closed

Removed use of 'Magento\Framework\Filesystem' #10671

wants to merge 1 commit into from

Conversation

dverkade
Copy link
Member

Removed use of 'Magento\Framework\Filesystem', this reference is not used in this class, so can be safely removed.

Removed use of 'Magento\Framework\Filesystem', this reference is not used in this class, so can be safely removed.
@orlangur
Copy link
Contributor

There is no sense to process such kind of PR as all such occurrences may be removed automatically with php-cs-fixer. But such removal does not make sense by itself until we enforce there are no unused use statements anymore with static test.

@dverkade
Copy link
Member Author

dverkade commented Aug 28, 2017

Hi @orlangur,

Instead of just rejecting this issue, you could have given some positive feedback. Something like "Hey, thank you for noticing, the @magento-team is aware of this issue, which is not only present in this file, but many other files as well. All such occurences can be removed automatically with the php-cs-fixer. But such removal does not make sense by itself until we enforce there are no unused use statements anymore with static test. Maybe we can create an issue for this and have it in the 'up for graps' project board on Github. Is this something you might be interested in to work on?"

Now this PR has been closed, but the actual issue has not been resolved. There are a lot of loose ends in the Magento code which should be cleaned up.

@orlangur
Copy link
Contributor

Hello @dverkade,

Sorry, just didn't want to look insincere. All this stuff when you close a ticket but say a couple of polite words, you know.

I do appreciate your observation as good-looking code is my fad. Just that automation is my bigger fad :)

Approach was described in general in one of your older PRs but didn't get any feedback: #7166 (comment). In this PR I gave enough information to proceed with appropriate implementation, feel free to ask any questions if it still seems vague to you.

the actual issue has not been resolved

I keep an eye on it, don't worry. Should be solved by the end of this year, not too much spare time these days unfortunately.

@dverkade dverkade deleted the patch-19 branch June 22, 2018 14:55
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