magento-hackathon / magento-composer-installer

Composer installer for Magento modules
210 stars 154 forks source link

Gitignore with globs doesn' work #154

Closed AydinHassan closed 9 years ago

AydinHassan commented 9 years ago

For example the following map just creates an entry in the .gitignore file as shell which is not right.

You can use this test to verify:

//MagentoHackathon\Composer\Magento\GitIgnoreGeneratorTestModule

public function testGitIgnoreWithWildCardMapping()
{
    $gitIgnoreFile = $this->getGitIgnoreTestPath();
    $map = array(
        array('shell/*.php', 'shell/'),
    );
    $package = $this->createPackageMock(array('map' => $map, 'auto-append-gitignore' => true));
    $this->composer->setPackage($package);
    $installer = new ModuleInstaller($this->io, $this->composer);
    $installer->appendGitIgnore($package, $gitIgnoreFile);

    $this->assertFileExists($gitIgnoreFile);
    $expectedContent = sprintf("#%s\n/shell/file1.php\n/shell/file2.php", $package->getName());
    $this->assertSame(file_get_contents($gitIgnoreFile), $expectedContent);
    unlink($gitIgnoreFile);
}

The only way I can think of properly fixing this is to store all created files in a class property here

Then after this is processed, pass that list to the .gitignore generator.

Would that be an acceptable solution? It would require editing all the deploy strategy classes.

If it is I'll start working on it

Flyingmana commented 9 years ago

thats to much changes for a simple review or judgment yet.

I would prefer to do this step by step.

First do a PR with the changes for tracking the deployed files ( thats a feature I would really like, as I can reuse this for some other things) I would extend this even, to have the deployed files grouped by the package they are from.

Then you can build the PR for the globs on top of this. And I think the solution sounds good.

Maybe also think about the possibility of tracking removed files (you dont need to implement this yet)

AydinHassan commented 9 years ago

Sounds good, half of that's done already. I'll fix up the PR for just tracking deployed files hopefully later today, if not sometime in the next couple of days. I'll try to not spend all Christmas programming :christmas_tree: :computer:

AydinHassan commented 9 years ago

@Flyingmana I have created a branch with proper GitIgnore support. All working and unit-tested. I've created a simple Event Manager and injected this into the DeployManager. I like this approach as we don't have to put the git ignore code in any class it does not belong.

It is only attached if the config value is enabled. I've done the same with the debug logging. Now the DeployManager has no dependencies other than the EVM. It is not ties to the console either.

I've also added some tests to the Plugin class and should serve as a base for better coverage in that class.

Anyway, the code is here: https://github.com/AydinHassan/magento-composer-installer/compare/evm-gitignore?expand=1

Let me know what you think of the code, and if okay I'll PR

Flyingmana commented 9 years ago

thats very good work :+1:

AydinHassan commented 9 years ago

Awesome, PR incoming