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

Add method Store #24

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

Conversation

maitrepylos
Copy link
Member

Hello, for one of my projects, I use Registry and I added a method to pass a configuration array, and it's easy for me to do it like that rather than using the set() method every time.

@ghost ghost assigned maitrepylos Sep 8, 2018
@ghost ghost added the in progress label Sep 8, 2018
Copy link
Member

@vonglasow vonglasow left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your PR can you rebase it from master and also fix the conflict ;)

Registry.php Outdated
*/
public static function store (array $contain) {

if(false === is_array($contain))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not needed as it's already defined as type of argument.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok thanks

Registry.php Outdated
@@ -78,7 +71,7 @@ class Registry extends \ArrayObject {
public function __construct ( ) {

throw new Exception(
'Cannot instance the %s object. Use set, get, remove ' .
'Cannot instance the %s object. Use set, get,store, remove ' .
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add space before store ;)

@coveralls
Copy link

coveralls commented Sep 12, 2018

Coverage Status

Coverage decreased (-10.8%) to 68.182% when pulling b8414f5 on maitrepylos:master into f9cf4ea on hoaproject:master.

@vonglasow
Copy link
Member

@maitrepylos it's rebased now it's operational you can add unit tests to cover the case.

Thanks

@vonglasow
Copy link
Member

ping @maitrepylos do you need any help about it ?

@maitrepylos
Copy link
Member Author

ping @vonglasow Hi, are you asking me to run tests? I don't know where to start!

@vonglasow
Copy link
Member

it was to add some tests to cover your feature we can discuss about it on IRC to help you.

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

Successfully merging this pull request may close these issues.

None yet

3 participants