search

babenkoivan / elastic-migrations

Elasticsearch migrations for Laravel
MIT License
187 stars 32 forks source link

Move command constructor args to `handle()` #32

Closed stevebauman closed 2 years ago

stevebauman commented 3 years ago

Closes https://github.com/babenkoivan/elastic-migrations/issues/30

I tried to match your code style as best I could when updating the tests, but they may need some adjustment to suit your preferences.

Let me know if you have any comments/questions/concerns.

No hard feelings on closure ❤️

stevebauman commented 3 years ago

The updated tests fail static analysis due to PHPUnit's mock objects. Should we set the analyzer to ignore the tests/* folder?

babenkoivan commented 3 years ago

The updated tests fail static analysis due to PHPUnit's mock objects. Should we set the analyzer to ignore the tests/* folder?

I think we just need to write the tests a bit differently, I left a comment with an example.

stevebauman commented 3 years ago

Great idea @babenkoivan 👍 , I've reset the tests and have done what you've requested. We're now all green.

babenkoivan commented 2 years ago

Thank you @stevebauman! I've just released v1.6.2, which includes your changes 🎉

stevebauman commented 2 years ago

Awesome, happy to help @babenkoivan! Appreciate it! 👍 Thanks for pushing it to a new release 🎉