TIPOFF / addresses

Laravel Package to manage addresses & interact with Address API
MIT License
0 stars 2 forks source link

add cities data migration #86

Closed arwaawan3 closed 3 years ago

arwaawan3 commented 3 years ago

still working on this..

drewroberts commented 3 years ago

I'm excited about this. Thank you for working on it.

arwaawan3 commented 3 years ago

@drewroberts So it looks like the timezones table only has some timezones, but the cities data has a lot more. I'm trying to handle the null values for timezone but the phpunit test is running out of memory.

arwaawan3 commented 3 years ago

Okay so I finally ended up splitting the city data migration into 3 files, each with about 33k records. I ran PHPUnit test with only the first migration and it took extremely long. I then added the others and pushed my changes. I think these migrations should be ignored by the tests otherwise, once we merge this branch into main, future tests will timeout because of these migrations.

Screen Shot 2021-03-22 at 6 17 05 PM
drewroberts commented 3 years ago

Let's make sure there are not Null values for the timezones. All the USA timezones are in the package migration, so those Cities should all have timezones present. We may need to change the PHP name for the timezones in either one of the migration so they match.

drewroberts commented 3 years ago

Great, I'm glad that worked with 3 files.

arwaawan3 commented 3 years ago

Ok, in that case I will update the data migrations. And the tests don't technically pass all together.. they just keep running.. It's probably because of the 100k records in the migration.

arwaawan3 commented 3 years ago

@drewroberts Ok I think this is finally done! But the migration tests still takes forever to complete.

drewroberts commented 3 years ago

That's fine. Could you just send a screenshot that they are running green?

Then I'm going to move it to the datamigrations folder which will not be migrated in testing environments and only when the package is installed in an application. @pdbreen created it that capability in this PR:

arwaawan3 commented 3 years ago

Yes they all complete and pass as long as you run one migration at a time. Here are the results.

cities01_test_pass cities02_test_pass cities03_test_pass
arwaawan3 commented 3 years ago

Closes #112