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 site_vars to templates/site_vars.html #17

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

calismu
Copy link

@calismu calismu commented Dec 2, 2024

Closes #13

@kjaymiller
Copy link
Collaborator

@calismu - LGTM

Let me get #18 figured out so I can test that the template builds as expected.

@john0isaac
Copy link
Member

@kjaymiller I think we agreed that it's better to separate the site_vars into a separate file and calling it settings.py as it's taking lots of space in the app.py

settings.py

SITE_VARS = {
...
} 

and in app.py
import SITE_VARS from settings

and app.site_vars.update(SITE_VARS)

This PR only separates them in rendering but after cookiecutter resolves it it's the same output.

@john0isaac john0isaac added the enhancement New feature or request label Dec 8, 2024
@kjaymiller
Copy link
Collaborator

@john0isaac - I think this can still be merged and we can add the additional structure in a separate PR.

@kjaymiller
Copy link
Collaborator

@calismu #19 was approved so this should be merged in the near future.

Copy link
Member

@john0isaac john0isaac left a comment

Choose a reason for hiding this comment

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

@calismu can you rebase your branch so the tests run? if the tests pass successfully we will merge the PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Status: Needs Tests
Development

Successfully merging this pull request may close these issues.

[Suggestion] Move the site_vars to a separate file
3 participants