magento-hackathon / magento-composer-installer

Composer installer for Magento modules
210 stars 156 forks source link

#97 Added prompt for missing magento root dir #99

Closed bragento closed 10 years ago

bragento commented 10 years ago

I just added the requested feature

There are no unit tests yet and it's basically just a first test. I'll gladly address any issues you report :)

Flyingmana commented 10 years ago

I dont like the automatic creation of the directory, as I have to expect the dumbest possible users. And there is a chance, they input a totally wrong path which could cause the install to land anywhere on the filesystem.

bragento commented 10 years ago

For me it's not about catching user stupidity but rather about minizing the setup overhead when using the magento composer installer. With this addition the extra info can automatically be added to the composer.json while the folder is automatically created exactly as defined. It leaves basically no space for mistakes and streamlines the initial project setup. Especially if we add the core install behavior. As for entering the wrong path, that could happen as well when entering the extra info manually. Since the code isn't executed for "normal" users anyhow I don't see any harm in adding it I guess.

tkdb commented 10 years ago

I'm undecided if this makes any sense to have. I would start with a command that is able to return that setting from JSON into STDOUT so that some shell script can deal with this or different needs easily (mkdir -p comes to mind). Just my 2 cents.

Flyingmana commented 10 years ago

FAILURES!
Tests: 104, Assertions: 141, Failures: 3, Errors: 48.

Biggest problem, its breaks the fullstack test, means it can/will break usage for existing users

bragento commented 10 years ago

Hi,

guess you were testing the version before my fix. Ok lease check the next rev again.

Greetz

Amir

On 02.06.2014, at 22:21, "Daniel Fahlke" notifications@github.com<mailto:notifications@github.com> wrote:

FAILURES!

Tests: 104, Assertions: 141, Failures: 3, Errors: 48.

Biggest problem, its breaks the fullstack test, means it can/will break usage for existing users

Reply to this email directly or view it on GitHubhttps://github.com/magento-hackathon/magento-composer-installer/pull/99#issuecomment-44885941.

Flyingmana commented 10 years ago

my fault, got a bit mixed up because I checked it out locally before