Skip to content
This repository has been archived by the owner on Nov 26, 2021. It is now read-only.

Update to L5.1 #25

Open
wants to merge 11 commits into
base: master
Choose a base branch
from
Open

Update to L5.1 #25

wants to merge 11 commits into from

Conversation

DODMax
Copy link

@DODMax DODMax commented Nov 21, 2015

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:
DevFactoryCH/media#8 (comment)

Cheers

Pulling latest changes from ipalaus/master
Seed only the Names, Countries and Continents tables
- Updated Readme
@ipalaus ipalaus mentioned this pull request Nov 23, 2015
@alexeyshockov
Copy link
Contributor

@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?

@DODMax
Copy link
Author

DODMax commented Nov 24, 2015

@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
Copy link
Author

DODMax commented Dec 5, 2015

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

See 42f170c

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

@yurtesen
Copy link

yurtesen commented Jul 2, 2016

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?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants