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 scripts for local servers #3

Closed
wants to merge 11 commits into from
Closed

Add scripts for local servers #3

wants to merge 11 commits into from

Conversation

urakagi
Copy link

@urakagi urakagi commented Oct 31, 2018

Add three scripts to set environment variables needed. Write down commands as script files.

@metasoarous
Copy link
Member

Thanks for putting this together. A couple notes:

  • Let's stick to the pattern in the rest of the repos of having a .env_dev file of environment variables that you source like source .env_dev. I actually don't like this pattern, and would like to move away from environment variables to config files for security concerns. But until we get there, let's stick with the existing pattern to avoid confusion.
  • There's no SERVER_PORT environment variable.
  • There's already a script for running the math worker in bin/run. Let's stick with this unless there's a good reason not to.
  • I'm somewhat nonplus on your wrapper for the export comand as a shell script; This command has a bunch of options, which become hidden if we coax people to learn this wrapped version. I'd rather just add some content to the README about how you call various subcommands of lein run.

@urakagi
Copy link
Author

urakagi commented Nov 1, 2018

Thank you @metasoarous,

  1. I super hate the environment variable so move to config files is very good for me.
  2. OK.
  3. I just don't understand the parameters of the original bin/run, so I just write a new script. Also, the worker reboot can't detect polisMath error and will be unstoppable.
  4. That will be a better solution, of course. After we moved to config files, this script can be erased.

@patcon
Copy link
Member

patcon commented Apr 10, 2020

ohai! is this PR still worth pulling anything out of, or should we close?

@patcon
Copy link
Member

patcon commented May 6, 2020

This command has a bunch of options, which become hidden if we coax people to learn this wrapped version. I'd rather just add some content to the README about how you call various subcommands of lein run.

👎 imho curious ppl who want to know about advanced flags know how to look into an executable :) simple binaries with defaults, and docs for the keeners. @urakagi's script looks exactly like what I'd expect, or alternatively hidden within a Makefile. cc @joshsmith2 (i think this is what you're looking for for data exports)

I'll submit another PR addressing your reco's as part of https://github.com/pol-is/polis-issues/issues/138, and close this one if that supercedes it

@patcon
Copy link
Member

patcon commented May 10, 2020

Closing this as part of repo transfer (https://github.com/pol-is/polis-issues/issues/134), but will cross-ref here so convo not lost: https://github.com/pol-is/polis-issues/issues/138

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