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

BASH-23 Add npm clean script #597

Merged
merged 1 commit into from
Sep 21, 2017
Merged

BASH-23 Add npm clean script #597

merged 1 commit into from
Sep 21, 2017

Conversation

csduarte
Copy link
Contributor

@csduarte csduarte commented Sep 8, 2017

Description
Adds a simple rm command to npm to remove all dynamically generated files to make it easy to reset between builds/branches

Test Cases
Run npm run clean

Copy link
Contributor

@yuya-oc yuya-oc left a comment

Choose a reason for hiding this comment

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

Just curious, is git clean -fdx not enough for you? And the files are already ignored by .gitignore. So I think they does not prevent switching branches.

package.json Outdated
@@ -20,6 +20,7 @@
"build:main": "cross-env NODE_ENV=production webpack --bail --config webpack.config.main.js",
"build:renderer": "cross-env NODE_ENV=production webpack --bail --config webpack.config.renderer.js",
"start": "electron src --disable-dev-mode",
"clean":"rm -rf dist/ release/ node_modules/ src/node_modules/ build/ browser/**/*_bundle.js}",
Copy link
Contributor

Choose a reason for hiding this comment

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

You should remove src/**/*_bundle.js instead of browser/**/*_bundle.js to remove src/main_bundle.js.

package.json Outdated
@@ -20,6 +20,7 @@
"build:main": "cross-env NODE_ENV=production webpack --bail --config webpack.config.main.js",
"build:renderer": "cross-env NODE_ENV=production webpack --bail --config webpack.config.renderer.js",
"start": "electron src --disable-dev-mode",
"clean":"rm -rf dist/ release/ node_modules/ src/node_modules/ build/ browser/**/*_bundle.js}",
Copy link
Contributor

Choose a reason for hiding this comment

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

  • missing a white space after clean:.
  • there is unexpected trailing token }.

@dmeza
Copy link
Contributor

dmeza commented Sep 11, 2017

@yuya-oc rebased to latest. Fixed all your comments built, packaged and run linting. Let me know if that's the correct change.

@yuya-oc
Copy link
Contributor

yuya-oc commented Sep 12, 2017

Thanks @dmeza! But sorry, I missed one thing. src/**/*_bundle.js might not enough at least on git-bash. Would you confirm whether src/browser/webview/mattermost_bundle.js is deleted?

@dmeza
Copy link
Contributor

dmeza commented Sep 12, 2017

@yuya-oc yeah, it did not work so I added these two: src/*_bundle.js and src/**/*_bundle.js and it cleans both directories src and src/browser.

package.json Outdated
@@ -20,6 +20,7 @@
"build:main": "cross-env NODE_ENV=production webpack --bail --config webpack.config.main.js",
"build:renderer": "cross-env NODE_ENV=production webpack --bail --config webpack.config.renderer.js",
"start": "electron src --disable-dev-mode",
"clean": "rm -rf dist/ release/ node_modules/ src/node_modules/ build/ src/*_bundle.js src/**/*_bundle.js",
Copy link
Contributor

Choose a reason for hiding this comment

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

Is rm -rf dist/ release/ node_modules/ src/node_modules/ build/ && find src -name '*_bundle.js' | xargs rm appropriate for you? This can remove src/browser/webview/mattermost_bundle.js

Copy link
Contributor

Choose a reason for hiding this comment

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

@dmeza What about this?

Copy link
Contributor

Choose a reason for hiding this comment

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

@yuya-oc changed and tested.

package.json Outdated
@@ -20,6 +20,7 @@
"build:main": "cross-env NODE_ENV=production webpack --bail --config webpack.config.main.js",
"build:renderer": "cross-env NODE_ENV=production webpack --bail --config webpack.config.renderer.js",
"start": "electron src --disable-dev-mode",
"clean": "rm -rf dist/ release/ node_modules/ src/node_modules/ build/ src/*_bundle.js src/**/*_bundle.js",
Copy link
Contributor

Choose a reason for hiding this comment

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

dist and build are not used in this project. Is it necessary explicitly removing them?

Copy link
Contributor

Choose a reason for hiding this comment

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

@yuya-oc: I thought I saw that some updates didn't get picked up because those folders already existed. If this is wrong, we should remove those from the clean task.

Copy link
Contributor

Choose a reason for hiding this comment

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

@dmeza Yeah, please remove. Those folders were actually used in past versions, so I think that you possibly saw them.

Copy link
Contributor

Choose a reason for hiding this comment

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

@yuya-oc removed dist and build folders from the clean script.

@jasonblais jasonblais added this to the v3.8.0 milestone Sep 15, 2017
@yuya-oc yuya-oc merged commit a9602c2 into mattermost:master Sep 21, 2017
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.

4 participants