Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add new scope based on class #54

Open
wants to merge 1 commit into
base: 3.x
Choose a base branch
from

Conversation

juanolon
Copy link

Add namespace option, when using the scope user, this way it is possible to register settings for different classes.

@Nyholm
Copy link
Collaborator

Nyholm commented Mar 12, 2016

For reference: This will fix #51

@Nyholm Nyholm mentioned this pull request Mar 12, 2016
@rvanlaak
Copy link
Owner

I'm not sure or naming it namespace is the correct way to go, sounds a bit confusing to me. What about naming it something like group or settingGroup or collection ? Can we also make sure there is a testcase for this? 🚀

@Nyholm
Copy link
Collaborator

Nyholm commented Mar 12, 2016

Lots of things have happened since you created this PR 5 hours ago. I think this is a great idea. @rvanlaak had some very good comments that we should address first.

@Nyholm Nyholm added this to the 2.0 milestone Mar 12, 2016
@rvanlaak
Copy link
Owner

@juanolon see #57 (comment)

@juanolon
Copy link
Author

i'm back. the original idea of calling it namespace was, that the value of the option is actually a class namespace. i'll continue on the other thread..

@juanolon
Copy link
Author

btw. testcases. i may take a little bit longer with that, as i'm not use to write tests on php..

@Nyholm
Copy link
Collaborator

Nyholm commented Mar 12, 2016

btw. testcases. i may take a little bit longer with that, as i'm not use to write tests on php..

Push something and we will help you.

Also, be aware that you need to rebase your work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants