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 custom settings into settings.py. #42

Merged
merged 2 commits into from
Jun 19, 2014
Merged

Conversation

rmnwolf
Copy link
Contributor

@rmnwolf rmnwolf commented Jun 18, 2014

This addresses issue 41.

@nibalizer
Copy link
Member

Why use so much file_line instead of a template? Puppet fully manages that file.

@rmnwolf
Copy link
Contributor Author

rmnwolf commented Jun 18, 2014

I felt that using file line resources would be a little cleaner than using a template which only adds relevant variables. Do you have a strong opinion on it?

@daenney
Copy link
Member

daenney commented Jun 18, 2014

@nibalizer does have a point. Since we're fully managing settings.py, not just parts of it using a template would be a better way to go than file_line.

I would also write settings.py one directory up, so inside $basedir/puppetboard and not in $basedir/puppetboard/puppetboard which contains the actual source checkout (doing so also means you need to modify the wsgi.py file again).

@rmnwolf
Copy link
Contributor Author

rmnwolf commented Jun 18, 2014

Okay. I'll refactor this into a template.

@rmnwolf
Copy link
Contributor Author

rmnwolf commented Jun 18, 2014

Refactored and forcepushed. How's that

@@ -0,0 +1,62 @@
<%
dev_listen_host = scope.lookupvar('puppetboard::dev_listen_host')
Copy link
Member

Choose a reason for hiding this comment

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

Just use @dev_listen_host, there is no reason to do a scope.lookupvar here. Oh i just got that you are setting variables here. There is no need to do that. Just use @variable down there in the template.

@nibalizer
Copy link
Member

Looks good. I got confused about how you were doing the template. I get it now.

@rmnwolf
Copy link
Contributor Author

rmnwolf commented Jun 18, 2014

Great! Happy to help!

MobileMail

On Jun 18, 2014, at 18:12, Spencer Krum [email protected] wrote:

Looks good. I got confused about how you were doing the template. I get it now.


Reply to this email directly or view it on GitHub.

This e-mail, including attachments, contains confidential and/or
proprietary information, and may be used only by the person or entity to
which it is addressed. The reader is hereby notified that any
dissemination, distribution or copying of this e-mail is prohibited. If you
have received this e-mail in error, please notify the sender by replying to
this message and delete this e-mail immediately.

@daenney
Copy link
Member

daenney commented Jun 19, 2014

@rmnwolf Could you remove all the scope.lookupvar things?

It should basically look like this:

<% if @dev_listen_host -%>
DEV_LISTEN_HOST = '<%= @dev_listen_host %>'
<% end -%>

There's no need to locally rebind them first. Don't forget to add the @ to the if's too.

@rmnwolf
Copy link
Contributor Author

rmnwolf commented Jun 19, 2014

done. How's that look?

daenney pushed a commit that referenced this pull request Jun 19, 2014
move custom settings into settings.py.
@daenney daenney merged commit 99927a3 into voxpupuli:master Jun 19, 2014
@rmnwolf rmnwolf deleted the Issue41 branch June 19, 2014 18:15
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