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 defaultBase for setting base for groups #1954

Merged
merged 7 commits into from
Sep 25, 2014

Conversation

tripp
Copy link
Contributor

@tripp tripp commented Sep 16, 2014

@caridy @ericf
This pull request adds unit tests to #1720

@yahoocla
Copy link

CLA is valid!

@caridy
Copy link
Member

caridy commented Sep 16, 2014

+1

@tripp
Copy link
Contributor Author

tripp commented Sep 16, 2014

@caridy
In your initial pr, you'd mention that base is king and will always take precedence when set. If defaultBase is set, base will only take precedence if its set on the group. I think this is fine because this only arises if you explicitly set defaultBase and it can be overridden by explicitly setting base on the module. Setting base globally will not take precedence over defaultBase set globally. To have that happen, we'd probably have to set a flag.

@caridy
Copy link
Member

caridy commented Sep 16, 2014

I don't fully understand the question, here is what I know:

  • if base is set, in config, or in a group, it is king.
  • as today, base is NOT shared between different groups or core.
  • if base is set at the module level, it is king.
  • root is not shared or propagated between different groups or core.

what does this means?

  • defaultBase is inherited in groups (from core)
  • defaultBase (from group or core) + root (from group) defines base for group when needed (when no base is defined in group)
  • defaultBase (from core) + root (from core) defines base for core when needed (when no base is defined in core)

@tripp
Copy link
Contributor Author

tripp commented Sep 17, 2014

That sounds like what we've got. I think I just misinterpreted you initial comments.

@tripp tripp merged commit 67ca863 into yui:dev-3.x Sep 25, 2014
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