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

Replace bower with npm #1330

Merged
merged 2 commits into from
Mar 8, 2018
Merged

Replace bower with npm #1330

merged 2 commits into from
Mar 8, 2018

Conversation

LaurentGoderre
Copy link
Member

@LaurentGoderre LaurentGoderre commented Mar 1, 2018

package.json Outdated
"dependencies": {
"fast-json-patch": "github:wet-boew/JSON-Patch#wb1.0+1.1.4",
"jsonpointer.js": "^0.4.0",
"wet-boew": "LaurentGoderre/wet-boew#themes-npm"
Copy link
Member Author

Choose a reason for hiding this comment

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

Replace this with upstream WET when wet-boew/wet-boew#8305 is merged

Copy link
Member

@nschonni nschonni left a comment

Choose a reason for hiding this comment

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

Mostly, LGTM, but I didn't run it.
Few minor questions about the bower and versions

@@ -183,8 +176,10 @@ module.exports = (grunt) ->
# Metadata.
pkg: @file.readJSON "package.json"
themeDist: "dist/<%= pkg.name %>"
jqueryVersion: @file.readJSON "lib/jquery/bower.json"
jqueryOldIEVersion: @file.readJSON "lib/jquery-oldIE/bower.json"
jqueryVersion: grunt.file.readJSON(
Copy link
Member

Choose a reason for hiding this comment

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

I did something "fancy" here in the upstream, but we could always just hardcode it like the OldIE one

Copy link
Member Author

Choose a reason for hiding this comment

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

This works without hardcoding so it makes it easier to maintain

package.json Outdated
"devDependencies": {
"assemble-contrib-i18n": "~0.1.2",
"assemble-contrib-i18n": "0.1.*",
Copy link
Member

Choose a reason for hiding this comment

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

Not sure I get this change. Wouldn't these be equivalant, or is the ~ a problem because it hasn't hit a major version yet?

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm just making it consistent with WET core

script/setup Outdated
@@ -1,2 +1,2 @@
#!/bin/sh
npm install -g bower grunt-cli && npm install
npm install -g grunt-cli && npm install
Copy link
Member

Choose a reason for hiding this comment

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

Is bower still needed for because of the prepare script in WET?

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh yeah it is for core.....But we can change core to not use it at all no?

@LaurentGoderre LaurentGoderre merged commit 6bbf08d into wet-boew:master Mar 8, 2018
@LaurentGoderre
Copy link
Member Author

@duboisp someone needs to a[pply this to all the other supported themes.

@LaurentGoderre LaurentGoderre changed the title WIP: Replace bower with npm Replace bower with npm Mar 8, 2018
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.

None yet

2 participants