quran / quran.com-frontend

quran.com frontend
https://quran.com
MIT License
994 stars 360 forks source link

Readme is missleading #531

Closed hellowin closed 7 years ago

hellowin commented 7 years ago

This part

Unless you have the backend API running locally, you will need to update the API_URL, in development.env file, from localhost to api.quran.com. Leave the port number same.

To start the app, run npm run dev which will run both the server and the client (webpack) to compile upon edits. Go to http://localhost:8001 in your browser, not 8000 (that is just the express server).

Is no longer relevant. Some updates are:

  1. development.env is now .env
  2. from localhost to api.quran.com is already API_URL=http://staging.quran.com:3000
  3. http://localhost:8001 become http://localhost:8000 now

Before I create PR, is my points are true?

mmahalwy commented 7 years ago

You are certainly correct except the 8001. Not on my computer but pretty sure it's that and not 8000 which will not have hot reloading

hellowin commented 7 years ago

Hmmm wait, I got these:

screen shot 2016-12-17 at 6 35 58 am

screen shot 2016-12-17 at 6 36 07 am

I haven't change anything yet. And look at these config:

  1. https://github.com/quran/quran.com-frontend/blob/master/.env#L2
  2. https://github.com/quran/quran.com-frontend/blob/master/webpack/webpack-dev-server.js#L11
  3. https://github.com/quran/quran.com-frontend/blob/master/package.json#L17

Number 1. is not used? number 2 will get port 8000 because it's hardcoded on package.json (number 3), is it as expected?

mmahalwy commented 7 years ago

+1 thanks!

ysahbi commented 7 years ago

Can you please specify that these steps does not work on Windows.

mmahalwy commented 7 years ago

sorry, what does not work?

ysahbi commented 7 years ago

I forked the project in both Ubuntu and Windows environnement and for both I ran npm run dev for Ubuntu it works, for windows i got this image

mmahalwy commented 7 years ago

hmm i think the problem is with env and probably should be export I think for windows. Maybe we should use something else for env variables in command line. What to try moving those variables to .env and see if it works?

thabti commented 7 years ago

Maybe cross-env can help?

mmahalwy commented 7 years ago

@sabeurthabti +!