Skip to content

Conversation

tperunsky
Copy link
Contributor

@tperunsky tperunsky commented Jan 4, 2022

#73

This PR makes the output look like this:
image

@@ -110,6 +110,10 @@ public function copyFile(string $source, string $target, array $options)

private function removeFilesFromDir(string $source, string $target)
{
if (!is_dir($source)) {
$this->write(sprintf(' Skipped directory <fg=green>"%s"</>: unable to determine which files to remove', $this->path->relativize($target)));
Copy link
Member

Choose a reason for hiding this comment

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

I'd suggest removing the message as I don't see what it would be useful for.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I removed it. I still think it would be nice to have it at least when running flex with -vvv, but that would require even more code changes, so perhaps it's best to keep it simple at this point. From my point of view the use case is developers setting up a private flex recipe. The documentation about doesn't go into too much detail about how flex works and I think that having extra information on why a directory was not removed during unconfiguration would save users from unnecessary confusion (and frustration). Alternatively, the documentation could explain in more detail what unconfiguration does and doesn't do. The naive expectation is that it completely cleans up, but that's not the reality (for good reasons). Either way I don't feel strongly about it.

@nicolas-grekas nicolas-grekas changed the base branch from 2.x to 1.x January 27, 2022 09:18
@nicolas-grekas nicolas-grekas force-pushed the unconfigure-skip-missing-source-dirs branch from d89a5e6 to 776319c Compare January 27, 2022 09:18
@nicolas-grekas
Copy link
Member

Thank you @tperunsky.

@nicolas-grekas nicolas-grekas merged commit d1ca6ac into symfony:1.x Jan 27, 2022
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.

2 participants