Skip to content

Conversation

@calismu
Copy link
Contributor

@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

@calismu calismu force-pushed the enhancement/move_site_vars_out branch from 5600684 to 606649c Compare April 27, 2025 17:37
@john0isaac john0isaac merged commit 043fbbe into render-engine:main Apr 27, 2025
4 checks passed
@github-project-automation github-project-automation bot moved this from Needs Tests to Done in Render Engine Apr 27, 2025
@calismu calismu deleted the enhancement/move_site_vars_out branch April 27, 2025 18:14
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: Done

Development

Successfully merging this pull request may close these issues.

[Suggestion] Move the site_vars to a separate file

3 participants