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

Move settings bundle to correct location (even in fallback cases) #197

Closed
wants to merge 1 commit into from
Closed

Move settings bundle to correct location (even in fallback cases) #197

wants to merge 1 commit into from

Conversation

bitcrumb
Copy link
Contributor

#195 moved the Resources group down into the project_namegroup. Code has been adapted to take this into account when moving the Settings.bundlearound.

Also fixed an issue where the file would be generated in the wrong location of one of the fallback parent_groups would be used.

@gfontenot
Copy link
Member

Looking at this now, the amount of code needed for this fix is disconcerting. I'm wondering if we shouldn't re-think how this works so that we can simply specify this location in the template yaml.

@bitcrumb
Copy link
Contributor Author

bitcrumb commented Dec 5, 2014

You're absolutely right, I'm not too happy about this code either, but it does fix it for now. Let me ponder on it some more. I like your suggestion that it should be configurable, that should make things more concise.

@gfontenot
Copy link
Member

@bitcrumb Any new thoughts here?

@bitcrumb
Copy link
Contributor Author

bitcrumb commented Feb 4, 2015

I guess specifying the location would be the simplest solution, with the main_group as a fallback. But I must admit that I lost track of this pull request. Will try to follow up on this in the coming days!

@gfontenot
Copy link
Member

Does this PR fix the current problem of this being broken? If so, maybe we just push forward with this and create an issue to simplify this in the future.

@bitcrumb
Copy link
Contributor Author

bitcrumb commented Feb 4, 2015

Yes it does. I'll let it up to you decide.

@gfontenot
Copy link
Member

👍 I've rebased and merged this as fda83f4

I'll create an issue to track simplifying this.

@gfontenot gfontenot closed this Feb 4, 2015
@bitcrumb bitcrumb deleted the feature/settings-fix branch February 16, 2015 21:26
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.

2 participants