Skip to content
This repository has been archived by the owner on Jan 5, 2019. It is now read-only.

Some actual code #1

Merged
merged 13 commits into from Jul 22, 2015
Merged

Some actual code #1

merged 13 commits into from Jul 22, 2015

Conversation

rosskarchner
Copy link
Member

Most notable for URL configuration, for mapping URL's to django-sheerlike views

rendering cfgov-refresh pages with django-sheerlike
Move the GlobalRequestMiddleware up the stack
- using the single SheerTemplateView generic view
- no longer need to spell out the inclusion of _includes and _layouts from a Sheer site
- no longer need to specify static dirs, if they are coming from a Sheer site
},
},
{
'BACKEND': 'django.template.backends.jinja2.Jinja2',

Choose a reason for hiding this comment

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

Do we not want this before the DjangoTemplates engine? Wouldn't this cause this to always try django templating first and then fall back on Jinja2?

Copy link
Member Author

Choose a reason for hiding this comment

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

That's a good point-- you're probably right

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed in a5ee37f

@kurtrwall
Copy link

Not sure if this comment should be here or django-sheerlike but currently django cannot access the templates in cfgov-refresh because the templates are now located in src folder (or dist after building). Adding the src folder to the path allows the templates to be found.


BASE_DIR = os.path.dirname(os.path.dirname(os.path.abspath(__file__)))

CFGOV_REFRESH = Path(__file__).ancestor(5).child('cfgov-refresh')

Choose a reason for hiding this comment

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

Why are we using ancestor instead of parent?

Choose a reason for hiding this comment

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

Also concerning my other comment, this could be CFGOV_REFRESH = Path(__file__).ancestor(5).child('cfgov-refresh').child('dist') to solve that problem.

Copy link
Member Author

Choose a reason for hiding this comment

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

because the equivalent of ancestor(5) would be parent.parent.parent.parent.parent? ;)

Copy link
Member Author

Choose a reason for hiding this comment

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

I think I'd prefer to keep CFGOV_REFRESH pointed at the top level project..., we can add .child('dist') in the SHEER_SITES setting.

Copy link
Member Author

Choose a reason for hiding this comment

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

Addressed in latest commit

Choose a reason for hiding this comment

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

Yeah I realized the parent() thing during lunch /facepalm 😟

Should we create a PROJECT_ROOT setting instead and then get the parent from that? Seems like we'd find putting the settings file where we want useful?

Copy link
Member Author

Choose a reason for hiding this comment

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

That might make sense for deployment scenarios, but I really think the simplest thing here is assuming both repos are sitting next to each other.

@rosskarchner
Copy link
Member Author

New feature: a 'gruntserver' command that extended 'runserver' to run 'grunt watch' in the background,

@@ -0,0 +1,6 @@
unipath>=1.1,<=2.0
django>=1.8,<=1.9
git+https://github.com/rosskarchner/django-sheerlike.git#egg=sheerlike

Choose a reason for hiding this comment

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

Do we want to keep you as the owner here? Just until your PR get's pulled in?

Copy link
Member Author

Choose a reason for hiding this comment

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

Just until the PR's get pulled in-- there's no code in the CFPB version of sheerlike yet.

Copy link
Member Author

Choose a reason for hiding this comment

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

Latest commit fixes this

@kurtrwall
Copy link

Depending on if the django-sheerlike PR gets merged before this, this is a-okay. If that PR does get merged, then I suggest changing the requirements.txt file to pull from the cfpb repo of django-sheerlike rather than yours before merging this.

@kurtrwall
Copy link

sheerlike is merged and this is ready to be updated.

@dpford
Copy link
Contributor

dpford commented Jul 22, 2015

👍 gruntserver is a nice touch, even if the update takes a while.

kurtrwall pushed a commit that referenced this pull request Jul 22, 2015
@kurtrwall kurtrwall merged commit b13e5e7 into cfpb:master Jul 22, 2015
@rosskarchner
Copy link
Member Author

thanks!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants