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 nodemon #161

Merged
merged 6 commits into from
Aug 11, 2019
Merged

Add nodemon #161

merged 6 commits into from
Aug 11, 2019

Conversation

servatj
Copy link
Contributor

@servatj servatj commented Aug 7, 2019

Adds nodemon as npm script related to the #156 issue

  • Remove Forever dependency as is not in use.
  • Add Nodemon as dependency
  • Add task dev to the package.json that allow us to reload the server on changes, see
    is not in use, let's move to Nodemon as npm script
  • Check that the npm tasks are working as expected
  • Update the readme.md with the extra relevant info (if needed)

@UlisesGascon UlisesGascon self-assigned this Aug 7, 2019
Copy link
Collaborator

@UlisesGascon UlisesGascon left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks a lot @servatj and welcome to the family ^^

I like that you also updated the Readme. 👍

I just re-started the Travis tests. I had some issue too before, like this one that I believe is a false-negative

Copy link
Member

@ckarande ckarande left a comment

Choose a reason for hiding this comment

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

@servatj Thanks for the PR and welcome to the project!

About code changes, when I tested it locally by starting the server using npm run dev, it seems nodemon is detecting changes only in the server.js but not in other js files or template files. Not sure if it was just my machine for some weird reason, but can you please confirm it is working as expected for you for all file types.

@servatj
Copy link
Contributor Author

servatj commented Aug 7, 2019

@ckarande Thanks for having me! You're totally right I misplaced the nodemon file in test folder, and also i removed the -watch option in package.json so nodemon gets the settings from the config properly.

@ckarande
Copy link
Member

ckarande commented Aug 8, 2019

@servatj Thanks for the changes. Looks good to me and we are all set to merge this PR. 🎉

@ckarande ckarande requested review from binarymist and lirantal and removed request for lirantal and binarymist August 8, 2019 15:39
@UlisesGascon UlisesGascon changed the base branch from master to release-1.5 August 9, 2019 06:10
@UlisesGascon
Copy link
Collaborator

Cool @ckarande @servatj :-) I just updated the PR base branch to OWASP:release-1.5 as expected

@ckarande
Copy link
Member

ckarande commented Aug 9, 2019

Awesome! @UlisesGascon Excellent work organizing the PRs with the right release branch. Please feel free to merge the PR when you get a chance.

@UlisesGascon
Copy link
Collaborator

Thanks @ckarande !! 👍

@UlisesGascon UlisesGascon merged commit 5bfc0d7 into OWASP:release-1.5 Aug 11, 2019
@UlisesGascon UlisesGascon mentioned this pull request Aug 11, 2019
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants