search

auraphp / Aura.Di

Dependency Injection System
MIT License
349 stars 63 forks source link

Replace each() with foreach() #172

Closed dstepe closed 5 years ago

dstepe commented 5 years ago

to prevent deprecation warnings under PHP 7.2

pmjones commented 5 years ago

@dstepe the Travis builds fail -- any chance you could look into those to see why?

pmjones commented 5 years ago

@dstepe also, if you happen to have time and inclination, let me know if something like this needs to be applied to the 3.x branch as well

dstepe commented 5 years ago

@pmjones Honestly, I had not intended to make the PR back to your repo. We have a project which uses the 2.x branch and we preparing to move it to PHP7.2. Due to lazy loading of some configuration, I can't use the 3.x branch of Aura.DI (I completely understand your reason for freezing the container). I wanted to address the PHP each function as a short term solution while we we figure out the longer term direction.

I'm happy to look at the Travis build for 2.x and get it working. If the 3.x branch still uses the each function, I'll be glad to make the change there as well.

dstepe commented 5 years ago

@pmjones Sorry for all the builds that hit travis. It was an interesting adventure to get as many PHP versions as possible working so there was a lot of updates to the travis config. I could not get the nightly hhvm to pass due to a name collision with a PHPUnit class. I added PHP 7.2 since that's the version i'm interested in.

I'm really glad I accidentally sent this PR to you. The failing tests revealed that my simple fix actually missed an important behavior. All the tests are now passing with no changes outside of that Factory method.

brandonsavage commented 5 years ago

I think this is an important PR to make this package forward-compatible.

pmjones commented 5 years ago

@dstepe thanks a ton for this!

pmjones commented 5 years ago

Hey all -- I just released 2.2.5 with this PR in place. Thanks for your patience!