Skip to content

Conversation

@dmendelowitz
Copy link
Contributor

Summary

You can now call npm start -- [options] to start the client instead of node src/cli/cli.js [options]

New behavior

There is now a script in package.json called "start" that acts as a simpler way of calling the program from the command line. It is important to note that npm requires you to put "--" between your script name and command line arguments. If the double hyphen is left out, the arguments will be interpreted as npm flags, for example -d and -f will be interpreted as npm's debug and force flags, not our debug and from date flags.

Code changes

  • package.json now has a start script, which runs the client
  • The README has been updated to include the new way of running the client

Testing guidance

Make sure that you are able to use the client as usual when calling via the start script

Copy link
Contributor

@julianxcarter julianxcarter left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks great and works really well for me! I have two small suggestions that I'd like to hear your thoughts on.

README.md Outdated
node src/cli/cli.js --help
```

The client can also be run by npm script:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if it'd be a good idea to entirely remove the instructions in the readme for running the client by way of the node src/cli/cli.js command. It seems like something that could only introduce confusion to some of our users, given that the node command only works from the top-level directory but the npm start command works from anywhere in the project. I'd love to hear your and @Dtphelan1's thoughts on this .

If we do keep the instructions for running the client with the node src/cli/cli.js command, we may want to move the npm start instructions to be the first command described in this readme and kind of suggest to users that this is our recommended way for running the client.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I wasn't sure if this was meant to be a replacement to node src/cli/cli.js or just an extra option. If we want to push this as the main way to run the client I can definitely make changes to the README to reflect that. I would like to hear other's opinions as well though.

Copy link
Contributor

@julianxcarter julianxcarter left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me!

@julianxcarter julianxcarter merged commit b9471c2 into develop Apr 16, 2021
@julianxcarter julianxcarter deleted the npm-start-script branch April 16, 2021 18:49
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