Skip to content
This repository was archived by the owner on Nov 14, 2019. It is now read-only.

Clean up Symfony-related stuff after creating the project #103

Closed
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
69 changes: 63 additions & 6 deletions src/Symfony/Installer/NewCommand.php
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ protected function execute(InputInterface $input, OutputInterface $output)
->cleanUp()
->updateParameters()
->updateComposerJson()
->createGitIgnore()
->checkSymfonyRequirements()
->displayInstallationResult()
;
Expand Down Expand Up @@ -348,14 +349,31 @@ private function extract()

/**
* Removes all the temporary files and directories created to
* download and extract Symfony.
* download the project and removes Symfony-related files that don't make
* sense in a proprietary project.
*
* @return NewCommand
*/
private function cleanUp()
{
$this->fs->remove(dirname($this->compressedFilePath));

try {
$licenseFile = array($this->projectDir.'/LICENSE');
$upgradeFiles = glob($this->projectDir.'/UPGRADE*.md');
$changelogFiles = glob($this->projectDir.'/CHANGELOG*.md');
Copy link
Member

Choose a reason for hiding this comment

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

@javiereguiluz What do you think about *.md then? You will probably not want to keep any markdown files from the SE at all.

Copy link
Member Author

Choose a reason for hiding this comment

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

We want to maintain for example the README.md file (but changing its contents). Removing all the .md files seems too farsighted for me.

Copy link
Member

Choose a reason for hiding this comment

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

If present, a readme would we overriden at all right now, wouldn't it?

Copy link
Member Author

Choose a reason for hiding this comment

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

Honestly, I'm not comfortable removing any *.md file as suggested. I prefer to be careful and remove the files we know for certain that should be removed (CHANGELOG, UPGRADE, LICENSE).


$filesToRemove = array_merge($licenseFile, $upgradeFiles, $changelogFiles);
$this->fs->remove($filesToRemove);

$readmeContents = sprintf("%s\n%s\n\nA Symfony project created on %s.\n", $this->projectName, str_repeat('=', strlen($this->projectName)), date('F j, Y, g:i a'));
$this->fs->dumpFile($this->projectDir.'/README.md', $readmeContents);
} catch (\Exception $e) {
// don't throw an exception in case any of the Symfony-related files cannot
// be removed, because this is just an enhancement, not something mandatory
// for the project
}

return $this;
}

Expand Down Expand Up @@ -476,12 +494,51 @@ private function updateComposerJson()
return $this;
}

$ret = str_replace(
'"name": "symfony/framework-standard-edition",',
sprintf('"name": "%s",', $this->generateComposerProjectName()),
file_get_contents($filename)
$contents = json_decode(file_get_contents($filename), true);

$contents['name'] = $this->generateComposerProjectName();
$contents['license'] = 'proprietary';

if (isset($contents['description'])) {
unset($contents['description']);
}

if (isset($contents['extra']['branch-alias'])) {
unset($contents['extra']['branch-alias']);
}

file_put_contents($filename, json_encode($contents, JSON_PRETTY_PRINT | JSON_UNESCAPED_SLASHES)."\n");

return $this;
}

/**
* Creates the appropriate .gitignore file for a Symfony project.
*
* @return NewCommand
*/
private function createGitIgnore()
{
$gitIgnoreEntries = array(
'/app/bootstrap.php.cache',
'/app/cache/*',
'!app/cache/.gitkeep',
'/app/config/parameters.yml',
'/app/logs/*',
'!app/logs/.gitkeep',
'/app/phpunit.xml',
'/bin/',
'/composer.phar',
Copy link
Member

Choose a reason for hiding this comment

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

What's the reason for the two entries above (/build/ and /composer.phar)?

Copy link

Choose a reason for hiding this comment

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

I guess because there are in the original .gitignore

Copy link
Member Author

Choose a reason for hiding this comment

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

@romqin is right, I copied the original .gitignore of the Standard Edition.

Copy link
Member

Choose a reason for hiding this comment

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

I think we should remove them here. Otherwise, we could even simply keep the one from the standard edition.

Copy link
Member Author

Choose a reason for hiding this comment

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

@xabbuh I'm afraid I don't understand you. Do you think we should remove this createGitIgnore() method from the installer?

Copy link
Member

Choose a reason for hiding this comment

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

I only mean the two entries /build/ and /composer.phar (the latter could be argued if we expect people to install Composer locally).

Copy link
Member Author

Choose a reason for hiding this comment

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

You are right. I've removed the strange /build/ entry, but left the composer.phar entry.

'/vendor/',
'/web/bundles/',
);
file_put_contents($filename, $ret);

try {
$this->fs->dumpFile($this->projectDir.'/.gitignore', implode("\n", $gitIgnoreEntries)."\n");
} catch (\Exception $e) {
// don't throw an exception in case the .gitignore file cannot be created,
// because this is just an enhancement, not something mandatory for the project
}

return $this;
}
Expand Down