geonames icon indicating copy to clipboard operation
geonames copied to clipboard

Update to L5.1

Open DODMax opened this issue 9 years ago • 4 comments

Hi,

If you're interested in reviewing the following changes as I know you're working on updating to 5.1

The migration part would need to be reworked as having the migrations files in the vendor folder creates warnings when running composer dump-autoload I read some people use blade template to store the migrations content: https://github.com/DevFactoryCH/media/issues/8#issuecomment-139564014

Cheers

DODMax avatar Nov 21 '15 12:11 DODMax

@DODMax, can you attach the command output? I don't get any warnings about migrations:

$ composer dump-autoload
Generating autoload files

And why _table postfix makes migrations Laravel 5.1 compliant? What's wrong with original files?

alexeyshockov avatar Nov 23 '15 20:11 alexeyshockov

@alexeyshockov, indeed the _table was needed because the commit I forked changed the migration classes names to add _table. Keeping both the filename and the classname without _table should also work and be backward compatible.

Here is the composer warning that is raised when running dump-autoload if the migrations are installed:

$ composer dump-autoload
Generating autoload files
Warning: Ambiguous class resolution, "CreateGeonamesNamesTable" was found in both "$baseDir . '/database/migrations/2013_11_28_170337_create_geonames_names_table.php" and "D:\dev\xampp\htdocs\socialc\vendor\ipalaus\geonames\src\migrations\2013_11_28_170337_create_geonames_names_table.php", the first will be used.

[...] (repeated for each migration file)

DODMax avatar Nov 24 '15 06:11 DODMax

Actually the duplicated class names warning can be fixed by removing the "src/migrations" from the autoload classmap in composer.json

See 42f170c604cd314a9c451b7f079336386250449c

Not sure how it plays with the test but I don't think the migrations are needed outside of Laravel.

DODMax avatar Dec 05 '15 09:12 DODMax

I removed the src/migrations from vendor/ipalaus/geonames/composer.json, but I am still seeing the "Warning: Ambiguous class resolution" when running dumpautoload, any ideas?

yurtesen avatar Jul 02 '16 13:07 yurtesen