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

Added script for fixing var_dir issues for images #2995

Open
wants to merge 19 commits into
base: 6.13
Choose a base branch
from

Conversation

mateuszbieniek
Copy link
Contributor

@mateuszbieniek mateuszbieniek commented Mar 18, 2020

This is port of legacy script that fixes images var_dir issues.

This PR is 6.13 version of #2638

mateuszbieniek and others added 8 commits March 18, 2020 14:15
…agesVarDirCommand.php

Co-Authored-By: Konrad Oboza <34310128+konradoboza@users.noreply.github.com>
…agesVarDirCommand.php

Co-Authored-By: Konrad Oboza <34310128+konradoboza@users.noreply.github.com>
Co-Authored-By: Konrad Oboza <34310128+konradoboza@users.noreply.github.com>
Co-Authored-By: Konrad Oboza <34310128+konradoboza@users.noreply.github.com>
…agesVarDirCommand.php

Co-Authored-By: Konrad Oboza <34310128+konradoboza@users.noreply.github.com>
…rage.php

Co-Authored-By: Konrad Oboza <34310128+konradoboza@users.noreply.github.com>
…rage.php

Co-Authored-By: Konrad Oboza <34310128+konradoboza@users.noreply.github.com>
Copy link
Member

@alongosz alongosz left a comment

Choose a reason for hiding this comment

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

I have a very general question. What is the expected usage of this command? Is it often, rare but can occur many times in the future or just one case during migration from eZ Publish?
Asking for two reasons. The first one is that EzPublishMigrationBundle has been dropped for eZ Platform 3.0, so you might get a surprise on a merge up.

The second reason is that if this is for regular maintenance, we need some integration test coverage (meaning those executed on real databases in integration suite). I could allow doing integration on gateway directly (usually it's on API), but we need to figure out the best place for such tests as this hasn't been done yet. TBH to ensure stability, most of this code (minus console output and I/O operation should be extracted to a separate service).

Why? Because when refactoring internal operations of Storage or even extracting Storage, I will for sure forget to test new methods.

But if this is just for one time migration then please disregard ;)

Anyway:

Comment on lines +141 to +142
* @param InputInterface $input
* @param OutputInterface $output
Copy link
Member

Choose a reason for hiding this comment

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

CS

Suggested change
* @param InputInterface $input
* @param OutputInterface $output

Comment on lines +225 to +231
/**
* Returns the number of recorded references outside of the given $path.
*
* @param string $uri File IO uri (not legacy)
*
* @return int
*/
Copy link
Member

Choose a reason for hiding this comment

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

CS: Please skip the doc, if the doc exists on parent (no inheritdoc needed also)

mateuszbieniek and others added 9 commits April 2, 2020 10:03
Co-Authored-By: Andrew Longosz <alongosz@users.noreply.github.com>
…agesVarDirCommand.php

Co-Authored-By: Andrew Longosz <alongosz@users.noreply.github.com>
…agesVarDirCommand.php

Co-Authored-By: Andrew Longosz <alongosz@users.noreply.github.com>
…agesVarDirCommand.php

Co-Authored-By: Andrew Longosz <alongosz@users.noreply.github.com>
…torage.php

Co-Authored-By: Konrad Oboza <34310128+konradoboza@users.noreply.github.com>
…shMigrationExtension.php

Co-Authored-By: Andrew Longosz <alongosz@users.noreply.github.com>
…torage.php

Co-Authored-By: Konrad Oboza <34310128+konradoboza@users.noreply.github.com>
…abase.php

Co-Authored-By: Andrew Longosz <alongosz@users.noreply.github.com>
Comment on lines +32 to +35
/**
* @var \eZ\Publish\Core\Persistence\Database\DatabaseHandler
*/
private $db;
Copy link
Member

Choose a reason for hiding this comment

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

Unused now? ;)

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

Successfully merging this pull request may close these issues.

5 participants