peter279k commented 5 years ago

Changed log

Using the __DIR__ to load current path of vendor/autoload.php without using the dirname function call and __FILE__ const.
Remove the throw $e syntax because it's unused.
To keep the condition easily, just use return isset($services[$key]); to return the boolean value directly.

XedinUnknown commented 4 years ago

Thanks for the contribution, and sorry for late reply!

Unfortunately, I cannot accept this PR in its current state.

It doesn't really make a difference whether you use dirname(__FILE__) or __DIR__. But in the long run, we will be shifting to a better bootstrapping pattern which uses an IIFE. The only argument to it will be the path to the root file, and the closure will determine everything else path-related based on that one value. So, dirname() will continue being used in the future, but not in the same way as now.
Didn't understand why you removed the callable typehint. IMHO it is the right type.

The other changes are good, however. If you would like us to consider this PR, please updated accordingly.

One thing is still a mystery to me, however: why suggest these very minor changes at all?

peter279k commented 4 years ago

Hi @XedinUnknown, thanks for your reply.

You're right. This is a minor changes and I think I can revert some code snippets you mention :).

XedinUnknown commented 4 years ago

Hi! I'm going to close this for now. If you still want to contribute, please let us know!

peter279k commented 4 years ago

Hi! I'm going to close this for now. If you still want to contribute, please let us know!

@XedinUnknown, thanks for your reply. Never mind. I'll create another PR when I figure some stuffs to improve this repository :).

Dhii / containers