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

update test command in package.json scripts area #3098

Closed
wants to merge 2 commits into from

Conversation

jsnellbaker
Copy link
Collaborator

Type of change

  • Build related changes

Description of change

As a follow-up from the gulp 4 upgrade (#2930), a deprecated gulp command was still present in the package.json scripts area.

I replaced the old gulp run-scripts command with the equivalent commands it used to run (gulp lint & gulp test-coverage).

Copy link
Member

@mkendall07 mkendall07 left a comment

Choose a reason for hiding this comment

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

But this doesn't run any tests?

@jsnellbaker
Copy link
Collaborator Author

I wanted to preserve the logic as it was in case anyone was using it. I can alter it to one of the following:

  • remove it outright from the scripts block
  • add in test command to the sequence
  • replace the two commands with the test command

Please let me know your thoughts.

@jaiminpanchal27
Copy link
Collaborator

I think we should remove scripts section from package.json completely because all our tasks are gulp based. Keeping just one in package.json does not make any sense.
Also test command is having 5 different flags. I am not sure how we can pass that to package.json

@mkendall07
Copy link
Member

I'm onboard with just removing scripts, or alternatively just having it run test gulp task (without any flags).

@jsnellbaker
Copy link
Collaborator Author

I just pushed a commit to remove the scripts block from the package.json file.

@jsnellbaker
Copy link
Collaborator Author

Closing in favor to #3117

@jsnellbaker jsnellbaker deleted the update_npm_scripts branch September 24, 2018 14:07
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