Skip to content
This repository was archived by the owner on Dec 5, 2019. It is now read-only.

refactor: apply webpack-defaults - #35

Merged
TheLarkInn merged 6 commits into
masterfrom
feature-defaults
Jun 29, 2017
Merged

refactor: apply webpack-defaults#35
TheLarkInn merged 6 commits into
masterfrom
feature-defaults

Conversation

@bebraw

@bebraw bebraw commented May 27, 2017

Copy link
Copy Markdown
Contributor

Closes #24.

@codecov

codecov Bot commented May 27, 2017

Copy link
Copy Markdown

Codecov Report

❗ No coverage uploaded for pull request base (master@0f58cd0). Click here to learn what that means.
The diff coverage is 0%.

Impacted file tree graph

@@          Coverage Diff           @@
##             master   #35   +/-   ##
======================================
  Coverage          ?    0%           
======================================
  Files             ?     2           
  Lines             ?   132           
  Branches          ?    46           
======================================
  Hits              ?     0           
  Misses            ?    91           
  Partials          ?    41
Impacted Files Coverage Δ
src/index.js 0% <0%> (ø)
src/cjs.js 0% <0%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0f58cd0...0659f13. Read the comment docs.

@bebraw bebraw mentioned this pull request May 27, 2017
@bebraw
bebraw force-pushed the feature-defaults branch from a22daf7 to bcb1bfd Compare June 6, 2017 07:48

@michael-ciniawsky michael-ciniawsky left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Latest webpack-defaults is the only obvious task left :) (yarn -> npm@5), if we land it sooner then later with the latest uglify@2 it can substitute lib/optimize/UglifyJSPlugin in webpack 3 as the (hopefully) last uglify@2 release

@bebraw

bebraw commented Jun 7, 2017

Copy link
Copy Markdown
Contributor Author

I updated it to webpack-defaults 1.1. I don't mind bumping later again.

The only thing I noticed is that it overrode ESLint completely. I had to disable a couple of rules to accommodate to the project so those were lost. Is this by design?

@Jessidhia

Copy link
Copy Markdown

Yes, webpack-defaults always overrides the eslint settings; presumably to install new rules if any are added to defaults.

Not sure if it's intentional that it doesn't support customizations at all.

@bebraw

bebraw commented Jun 7, 2017

Copy link
Copy Markdown
Contributor Author

@Kovensky Ok, track webpack-contrib/webpack-defaults#59 .

@bebraw
bebraw force-pushed the feature-defaults branch 2 times, most recently from d295a5e to 23733c2 Compare June 10, 2017 07:42

@joshwiens joshwiens left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

  • Delete the yarn.lock & replace with package-lock.json

Comment thread appveyor.yml
matrix:
fast_finish: true
install:
- 'ps: Install-Product node $env:nodejs_version x64'

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

You have to strip the ' off of anything that has ps: || cmd:. Already has an open issue in defaults.

Comment thread .travis.yml
@@ -1,10 +1,34 @@
sudo: false

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pickup webpack-defaults@1.4.0 for all the psudo canary Travis changes.

Comment thread package.json Outdated
"eslint-plugin-node": "^4.0.1",
"eslint-plugin-react": "^6.9.0",
"git-prepush-hook": "^1.0.1",
"jest": "^18.1.0",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Change to "jest": "^20.0.4",,

Comment thread package.json Outdated
@@ -56,23 +69,40 @@
"babel-jest": "^18.0.0",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Change to "babel-jest": "^20.0.3",

Comment thread package.json Outdated
"babel-preset-es2015": "^6.18.0",
"cross-env": "^5.0.0",
"del-cli": "^1.0.0",
"eslint": "^3.13.1",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Strip the eslint deps in favor of ..

    "eslint": "^3.19.0",
    "eslint-config-webpack": "^1.2.3",
    "eslint-plugin-import": "^2.3.0",

Comment thread package.json Outdated
"eslint-plugin-jsx-a11y": "^3.0.2",
"eslint-plugin-node": "^4.0.1",
"eslint-plugin-react": "^6.9.0",
"git-prepush-hook": "^1.0.1",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Shouldn't need this anymore?

Comment thread package.json
"babel-jest": "^18.0.0",
"babel-plugin-syntax-object-rest-spread": "^6.13.0",
"babel-plugin-transform-object-rest-spread": "^6.20.2",
"babel-polyfill": "^6.23.0",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Strip all the Babel in favor of ...

    "babel-cli": "^6.24.1",
    "babel-jest": "^20.0.3",
    "babel-plugin-transform-object-rest-spread": "^6.23.0",
    "babel-polyfill": "^6.23.0",
    "babel-preset-env": "^1.5.2",

Comment thread package.json Outdated
"sync-exec": "^0.6.2",
"uglify-js": "^2.8.18",
"webpack": "^2.2.0"
"webpack": "^2.2.0",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull the hard devDependency on webpack

@joshwiens

Copy link
Copy Markdown
Contributor

@Kovensky - webpack-defaults always overrides the eslint settings is very intentional.

The entire point of defaults was to standardize all of the loaders & plugins in contrib to simplify contributions & maintenance, this includes amongst other things, code style. We have a single way of doing things because in the end, it's always going to be a small group of people handling a vast majority of the project maintenance.

Rules can be overridden in the .eslintrc if it's necessary or a pull request can be opened in eslint-config-webpack for rules that should be changed across the board.

There is discussion about using Prettier in the future but as it stands right now, there is far more value in getting the standards work done than tinkering around with stylistic debates. We can take another stylistic pass, probably with prettier in webpac-defaults 2.x along with the schema-utils updates but until https://github.com/orgs/webpack-contrib/projects/5 board is cleared, we are going with good enough

@michael-ciniawsky michael-ciniawsky changed the title chore: Update to webpack-defaults refactor: apply webpack-defaults Jun 22, 2017
@michael-ciniawsky michael-ciniawsky added this to the 1.0.0 milestone Jun 22, 2017
@michael-ciniawsky michael-ciniawsky self-assigned this Jun 28, 2017
@michael-ciniawsky

michael-ciniawsky commented Jun 28, 2017

Copy link
Copy Markdown
Member

Updated to webpack-defaults v1.4.0 can I merge it so we can start backporting the test and update to uglify >= v3.0.0 ?

@michael-ciniawsky

Copy link
Copy Markdown
Member

Why is the CI not working? 😕 😭

@TheLarkInn
TheLarkInn merged commit 3bd2f1f into master Jun 29, 2017
@TheLarkInn

Copy link
Copy Markdown
Contributor

Shipit.

joshwiens pushed a commit that referenced this pull request Jun 29, 2017
* fix(package): mv uglify2 to `dependencies` && update `peerDependencies` (#45)

* refactor: apply to `webpack-defaults`

* chore(package): update `webpack-defaults v1.0.1...1.4.0`

* refactor(package): rm `pre-push` hook

* chore: rm `yarn.lock`

* fix(index): linting
joshwiens pushed a commit that referenced this pull request Jun 29, 2017
* fix(package): mv uglify2 to `dependencies` && update `peerDependencies` (#45)

* refactor: apply to `webpack-defaults`

* chore(package): update `webpack-defaults v1.0.1...1.4.0`

BREAKING CHANGE: Enforces `peerDependencies` of `"webpack": ">= 3.0.0-rc.0 || ^3.0.0"`.

BREAKING CHANGE: Enforces `engines` of `"node": ">=4.3.0 < 5.0.0 || >= 5.10`
@michael-ciniawsky michael-ciniawsky removed this from the 2.0.0 milestone Oct 29, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants