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.

Sign up for GitHub

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

Jump to bottom

Update build process for ES6 #320

Merged
merged 10 commits into from
Jan 12, 2017
Merged

Update build process for ES6 #320

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
9b5cb3a
Update build process for ES6
dbkr Jan 11, 2017
0dfd60a
Merge compile target into build
dbkr Jan 11, 2017
ff72398
Add exceptions to eslintrc for JSDoc
dbkr Jan 12, 2017
0a11404
Fix legitimate JSDoc errors
dbkr Jan 12, 2017
31af4bb
Fix jsdoc errors in spec/
dbkr Jan 12, 2017
7f8375d
Lint spec as well as src
dbkr Jan 12, 2017
591b56d
it's build now, not compile
dbkr Jan 12, 2017
543b9cf
Run lint on prepublish, not build
dbkr Jan 12, 2017
e057956
Add google eslint rules as a base
dbkr Jan 12, 2017
4ab261b
Add eslint:recommends
dbkr Jan 12, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions .eslintrc.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
module.exports = {
parserOptions: {
ecmaVersion: 6,
sourceType: "module",
ecmaFeatures: {
}
},
rules: {
"max-len": ["warn", {
code: 90,
ignoreComments: true,
}],
curly: ["error", "multi-line"],
}
Copy link
Member

Choose a reason for hiding this comment

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

should we enable http://eslint.org/docs/rules/require-jsdoc and http://eslint.org/docs/rules/valid-jsdoc? It would be a shame if we started slipping into bad habits with the jsdoc.

Copy link
Member Author

Choose a reason for hiding this comment

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

Interestingly, turning these on produces 537 warnings, so presumably it's somewhat stricter than jshint :/

Should we turn them on and aspire to fix the warnings?

}
11 changes: 0 additions & 11 deletions .jshint
View file
Open in desktop

This file was deleted.

9 changes: 2 additions & 7 deletions jenkins.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,8 @@ function fail {

npm test || fail "npm test finished with return code $?"

jshint --reporter=checkstyle -c .jshint src spec > jshint.xml ||
fail "jshint finished with return code $?"

gjslint --unix_mode --disable 0131,0211,0200,0222,0212 \
--max_line_length 90 \
-r src/ -r spec/ > gjslint.log ||
fail "gjslint finished with return code $?"
node_modules/.bin/eslint -f checkstyle src spec > eslint.xml ||
fail "eslint finished with return code $?"

# delete the old tarball, if it exists
rm -f matrix-js-sdk-*.tgz
Expand Down
8 changes: 4 additions & 4 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,12 @@
"test": "istanbul cover --report cobertura --config .istanbul.yml -i \"lib/**/*.js\" jasmine-node -- spec --verbose --junitreport --captureExceptions",
"check": "jasmine-node spec --verbose --junitreport --captureExceptions",
"gendoc": "jsdoc -r lib -P package.json -R README.md -d .jsdoc",
"compile": "babel -s -d lib src",
"compile": "npm run lint && babel -s -d lib src",
Copy link
Member

Choose a reason for hiding this comment

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

is there a reason to maintain a separate compile target? why not just stick it into build ?

Copy link
Member Author

Choose a reason for hiding this comment

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

wfm

"start": "babel -s -w -d lib src",
"build": "jshint -c .jshint lib/ && rimraf dist && mkdir dist && browserify --exclude olm browser-index.js -o dist/browser-matrix.js --ignore-missing && uglifyjs -c -m -o dist/browser-matrix.min.js dist/browser-matrix.js",
"build": "npm run compile && rimraf dist && mkdir dist && browserify --exclude olm browser-index.js -o dist/browser-matrix.js --ignore-missing && uglifyjs -c -m -o dist/browser-matrix.min.js dist/browser-matrix.js",
"dist": "npm run build",
"watch": "watchify --exclude olm browser-index.js -o dist/browser-matrix-dev.js -v",
"lint": "jshint -c .jshint src spec && gjslint --unix_mode --disable 0131,0211,0200,0222,0212 --max_line_length 90 -r spec/ -r src/",
"lint": "eslint src",
Copy link
Member

Choose a reason for hiding this comment

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

shouldn't we check the spec folder too? we used to, and jenkins.sh apparently does.

Copy link
Member

Choose a reason for hiding this comment

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

can we set --max-warnings on this?

Copy link
Member Author

Choose a reason for hiding this comment

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

fixed (and max-warnings now redundant since everything is an error)

"prepublish": "npm run compile && git rev-parse HEAD > git-revision.txt"
Copy link
Member

Choose a reason for hiding this comment

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

s/compile/build/

Copy link
Member Author

Choose a reason for hiding this comment

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

done

},
"repository": {
Expand Down Expand Up @@ -53,10 +53,10 @@
"devDependencies": {
"babel-cli": "^6.18.0",
"babel-preset-es2015": "^6.18.0",
"eslint": "^3.13.1",
"istanbul": "^0.3.13",
"jasmine-node": "^1.14.5",
"jsdoc": "^3.4.0",
"jshint": "^2.8.0",
"rimraf": "^2.5.4",
"uglifyjs": "^2.4.10",
"watchify": "^3.2.1"
Expand Down