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

React integration #11

Merged
merged 9 commits into from
Mar 17, 2019
Merged

React integration #11

merged 9 commits into from
Mar 17, 2019

Conversation

hyshka
Copy link
Contributor

@hyshka hyshka commented Mar 13, 2019

What do you two think of this first pass?

The Python code probably isn't up to your standards so definitely give me some feedback on how I can improve it.

Basically, I used Create React App to generate the initial code in /src/, then wrote my own webpack config to add Django integration with django-webpack-loader. I set it up for now so the app should be viewable at localhost:8000/app.

I didn't commit any built files, and didn't add Docker yet, but for this to be viewable you need to run the webpack server npm run start and then open it up at the Django URL.

Django webpack loader will append links to the generated webpack bundles in the index.html template. By default, for development, webpack will then try to load assets from localhost:3000 which is why you need the webpack server running.

@hyshka hyshka self-assigned this Mar 13, 2019
@ashiazed
Copy link
Member

⛷️ This looks really great. We'll probably have some rearranging of folders still but let's do that after this is merged. @thornycrackers will check this over and then let's merge it in.

This was referenced Mar 16, 2019
@hyshka
Copy link
Contributor Author

hyshka commented Mar 16, 2019

I cleaned up some code based off of @ashiazed 's suggestions here.

@hyshka
Copy link
Contributor Author

hyshka commented Mar 17, 2019

@thornycrackers can you review the Docker and Makefile integration for me? I'm curious what suggestions you have.

Right now for local development running both BE + FE is required.

I'd like to add CI for frontend linting, is the one make lint_frontend command enough to set that up with?

@thornycrackers
Copy link
Contributor

@hyshka It looks good. The only thing I'd get picky over is using bash over sh, and having the docker commands in a compose file vs using docker run but those can be cleaned up later I'll make some tasks for them later. I can look at adding the front end linting from what you've provided.

@thornycrackers thornycrackers merged commit 38c4d87 into master Mar 17, 2019
@thornycrackers thornycrackers deleted the react_integration branch March 17, 2019 15:29
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.

None yet

3 participants