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

Production config for Heroku #185

Open
wants to merge 18 commits into
base: master
Choose a base branch
from

Conversation

jlev
Copy link

@jlev jlev commented Oct 4, 2017

Hey folks, there were a few changes needed to get this building on Heroku. Also compressed the bundle significantly, ensured it would be able to cache properly, and fixed an issue with server-side redirects.

@lupitadavila
Copy link
Collaborator

lupitadavila commented Oct 13, 2017

the linter is blocking the tests, can you look into to them? @jlev not sure what your thoughts are on the linter rules that are set up right now

@jlev
Copy link
Author

jlev commented Oct 16, 2017

Hi Lupita, thanks for the reminder. I hadn't set up the linter to run on my git commits, so I hadn't seen this failing on Travis. I pushed the fixes recommended by eslint, and made a minor change to the rc to allow for plusplus when in for-loops.

I'm not a modern javascript expert, so I don't yet default to fat arrow syntax, but the linting rules seem fine to me and are a good reminder on a big shared project. There are a lot of warnings about console statements, which should probably be handled by a logging system going forward.

lupitadavila
lupitadavila previously approved these changes Oct 16, 2017
@lupitadavila
Copy link
Collaborator

@perfectlynormalbeast I'm not sure why the checks aren't syncing with this PR, do you have any idea? I reran the build and it passed https://travis-ci.org/Twilio-org/phonebank/jobs/282995539

@JadziaHax
Copy link
Contributor

@jlev you might want to try another push to this branch.

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