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

Add build-local.json check for local development #13

Closed
2 tasks done
mbarlow12 opened this issue May 23, 2018 · 9 comments
Closed
2 tasks done

Add build-local.json check for local development #13

mbarlow12 opened this issue May 23, 2018 · 9 comments
Assignees

Comments

@mbarlow12
Copy link
Contributor

mbarlow12 commented May 23, 2018

We've hardcoded the address of a local development. @zepumph noted in #12 that this should be configurable for different environments. To allow for this, we'll add a check in ~/.phet/build-local.json for the following keys:

{
    "localHostURL": "http://localhost",
    "localHostPort": "8080"
}

Since node's http-server runs the server configured above, those will be the defaults.

  • Add build-local.json check and defaults for setting the local dev server URL in getFromSimInMaster.js.
  • Add config options to README
@mbarlow12 mbarlow12 self-assigned this May 23, 2018
@zepumph
Copy link
Member

zepumph commented May 23, 2018

This would be so very nice. I would also recommending loud asserts if those keys are not present when running.

mbarlow12 added a commit that referenced this issue May 24, 2018
mbarlow12 added a commit to phetsims/perennial that referenced this issue May 24, 2018
@mbarlow12
Copy link
Contributor Author

mbarlow12 commented May 24, 2018

All that's left for this is to update the documentation. To generate the docs, binder looks in build-local.json for the following:

{
  "localhostURL": "https://someUrl",    // REQUIRED with asserts
  "localhostPort": "42"   // optional
}

It uses https://github.com/phetsims/perennial/blob/master/js/common/buildLocal.js to access the local configuration, so the getters have been added.

@samreid
Copy link
Member

samreid commented May 24, 2018

I don't mind adding localhostPort to my config, but I thought it would be nice to poll the development team to see how many people are using each port. Let's use this comment as the voting comment, with 👍 as 8080, 😆 as 80 and ❤️ as something else.

@jonathanolson
Copy link
Contributor

I'm not sure why the URL and port are separated, since the port could be part of the URL. How do you plan to handle http://localhost:8080/something/? It seems like it would be easier (and fixes this issue) to have it be part of one URL.

@mbarlow12
Copy link
Contributor Author

@jonathanolson That's true—I think that'd be a worthwhile change. It's just been a habit for a long time to separate the port.

@pixelzoom
Copy link

FYI, I'm using port 80 because it's the default for Apache on macOS. It can be configured in /etc/apache2/httpd.conf.

@mbarlow12
Copy link
Contributor Author

@pixelzoom do you see problems arising from using a single key in build-local.json for both the host and port?

mbarlow12 added a commit that referenced this issue May 24, 2018
mbarlow12 added a commit to phetsims/perennial that referenced this issue May 24, 2018
@pixelzoom
Copy link

I've been out of the loop wrt binder, so much so that I have no idea why a host and port are even needed. Try it and see?

@mbarlow12
Copy link
Contributor Author

README updated. Closing.

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

No branches or pull requests

5 participants