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

Remove empty code blocks #32692

Open
settermjd opened this issue Sep 13, 2018 · 3 comments
Open

Remove empty code blocks #32692

settermjd opened this issue Sep 13, 2018 · 3 comments

Comments

@settermjd
Copy link
Contributor

After reviewing the code, I've come across a number of empty code blocks that serve no purpose. Perhaps, at some earlier time, they were used but have gradually been trimmed down over time until they are now empty, yet never removed. Regardless, as they no longer serve any purpose, other than to cause confusion, it's best to remove them.

They are:

https://github.com/owncloud/core/blob/master/apps/files_sharing/lib/Helper.php#L143
https://github.com/owncloud/core/blob/master/lib/private/App/CodeChecker/NodeVisitor.php#L136
https://github.com/owncloud/core/blob/master/lib/private/Archive/ZIP.php#L46
https://github.com/owncloud/core/blob/master/lib/private/App/CodeChecker/NodeVisitor.php#L136
https://github.com/owncloud/core/blob/master/lib/private/App/CodeChecker/NodeVisitor.php#L164
https://github.com/owncloud/core/blob/master/lib/private/App/CodeChecker/NodeVisitor.php#L180
https://github.com/owncloud/core/blob/master/tests/data/app/code-checker/test-not-equal.php#L8
https://github.com/owncloud/core/blob/master/tests/data/app/code-checker/test-identical-operator.php#L8
https://github.com/owncloud/core/blob/master/tests/data/app/code-checker/test-identical-operator.php#L10
https://github.com/owncloud/core/blob/master/tests/data/app/code-checker/test-equal.php#L8
https://github.com/owncloud/core/blob/master/lib/private/Files/Storage/Common.php#L430

@ownclouders
Copy link
Contributor

GitMate.io thinks the contributor most likely able to help you is @PVince81.

Possibly related issues are #240 (--- Removed ---), #3260 (- Removed -), #31888 (removed dead code), #31889 ([stable10] remove dead code), and #28612 (Remove unused win code (#28609)).

@PVince81
Copy link
Contributor

all are harmless.

the code checker has its own test data with dummy classes, not code that actually runs.

The last one we'll need to address at some point: https://github.com/owncloud/core/blob/master/lib/private/Files/Storage/Common.php#L430

From what I see this method was added by mistake and should have used normalizePath() instead:

  • Approach 1:
    • remove Common::cleanPath (it's not on any interface)
    • remove cleanPath usage in SFTP and make it use Filesystem::normalizePath() instead
    • cons: in case other ext storages use it, they'll break
    • pros: cleaner classes...
  • Approach 2:

@emjrymer
Copy link

emjrymer commented Oct 3, 2019

I'd be happy to help clean this up!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment