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

[MakeDocker] add support for .yml docker-compose files #818

Merged
merged 1 commit into from
Apr 23, 2021

Conversation

jrushlow
Copy link
Collaborator

Currently, make:docker:database only supports creating & manipulating docker-compose.yaml files. In the docker world however, it is common to have docker-compose.yml files. This PR adds support to manipulate existing docker-compose(.yml||.yaml) files.

When running this command without an existing compose file, .yaml will be used.

fixes #777

@jrushlow jrushlow marked this pull request as ready for review February 18, 2021 07:09
@jrushlow jrushlow added the Status: Needs Review Needs to be reviewed label Feb 18, 2021
$composeFileContents = '';
$statusMessage = 'Existing docker-compose.yaml not found: a new one will be generated!';

if ($this->fileManager->fileExists($this->composeFilePath)) {
$composeFileContents = $this->fileManager->getFileContents($this->composeFilePath);
Copy link
Member

Choose a reason for hiding this comment

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

It looks like we currently calculate the $this->composeFilePath property in the constructor. Instead of doing that logic (and instead of having the inline logic here), could we remove that property and replace it with a private function getComposeFilePath(): ?string? That would loop over the possible paths and return the file path if it was found. Then, most of this section could stay the same, except that we would call the method instead of referencing the property.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We need access to that property in generate() to be able to dump the new / update content. The latest commit adds a getComposeFileContents() that determines the file && grabs its contents if it exists. I think this is as close as we can get before we start abusing input args.

We can revert the logic split back to inline if this implementation isn't as appetizing.

@weaverryan
Copy link
Member

Thanks Jesse!

@weaverryan weaverryan merged commit 2d90730 into symfony:main Apr 23, 2021
@jrushlow jrushlow deleted the feature/yml-docker-support branch April 25, 2021 16:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Needs Review Needs to be reviewed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Docker][database] the maker does not work because of the YamlSourceManipulator
2 participants