-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
add make targets for js and css, add js linter #6952
Changes from 8 commits
c6e1766
5b7f59a
4bd52d7
0679ef0
77b954b
3ebbfb1
119b725
eb7ecaf
8d9d179
43e3318
9cee9f8
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,25 @@ | ||
root: true | ||
|
||
extends: | ||
- eslint:recommended | ||
|
||
parserOptions: | ||
ecmaVersion: 2015 | ||
|
||
env: | ||
browser: true | ||
jquery: true | ||
es6: true | ||
|
||
globals: | ||
Clipboard: false | ||
CodeMirror: false | ||
emojify: false | ||
SimpleMDE: false | ||
Vue: false | ||
Dropzone: false | ||
u2fApi: false | ||
hljs: false | ||
|
||
rules: | ||
no-unused-vars: [error, {args: all, argsIgnorePattern: ^_, varsIgnorePattern: ^_, ignoreRestSiblings: true}] | ||
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -365,33 +365,46 @@ release-compress: | |
fi | ||
cd $(DIST)/release/; for file in `find . -type f -name "*"`; do echo "compressing $${file}" && gxz -k -9 $${file}; done; | ||
|
||
.PHONY: javascripts | ||
javascripts: public/js/index.js | ||
.PHONY: js | ||
js: | ||
@hash npx > /dev/null 2>&1; if [ $$? -ne 0 ]; then \ | ||
echo "Please install npm version 5.2+"; \ | ||
exit 1; \ | ||
fi; | ||
npx eslint public/js | ||
|
||
.IGNORE: public/js/index.js | ||
public/js/index.js: $(JAVASCRIPTS) | ||
cat $< >| $@ | ||
.PHONY: css | ||
css: | ||
@hash npx > /dev/null 2>&1; if [ $$? -ne 0 ]; then \ | ||
echo "Please install npm version 5.2+"; \ | ||
exit 1; \ | ||
fi; | ||
npx lesshint public/less/ | ||
npx -p less lessc --clean-css="--s0 -b" public/less/index.less public/css/index.css | ||
$(foreach file, $(filter-out public/less/themes/_base.less, $(wildcard public/less/themes/*)),npx -p less lessc --clean-css="--s0 -b" public/less/themes/$(notdir $(file)) > public/css/theme-$(notdir $(call strip-suffix,$(file))).css;) | ||
npx postcss --use autoprefixer --no-map --replace public/css/* | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Had some weird issue with the |
||
|
||
.PHONY: stylesheets-check | ||
stylesheets-check: generate-stylesheets | ||
@diff=$$(git diff public/css/*); \ | ||
if [ -n "$$diff" ]; then \ | ||
echo "Please run 'make generate-stylesheets' and commit the result:"; \ | ||
if ([ ! -z "$CI" ] && [ -n "$$diff" ]); then \ | ||
echo "Generated files in public/css have changed, please commit the result:"; \ | ||
echo "$${diff}"; \ | ||
exit 1; \ | ||
fi; | ||
|
||
.PHONY: javascripts | ||
javascripts: | ||
echo "'make javascripts' is deprecated, please use 'make js'" | ||
$(MAKE) js | ||
|
||
.PHONY: stylesheets-check | ||
stylesheets-check: | ||
echo "'make stylesheets-check' is deprecated, please use 'make css'" | ||
$(MAKE) css | ||
|
||
.PHONY: generate-stylesheets | ||
generate-stylesheets: | ||
@hash npx > /dev/null 2>&1; if [ $$? -ne 0 ]; then \ | ||
echo "Please install npm version 5.2+"; \ | ||
exit 1; \ | ||
fi; | ||
$(eval BROWSERS := "> 1%, last 2 firefox versions, last 2 safari versions, ie 11") | ||
npx lesshint public/less/ | ||
npx lessc --clean-css="--s0 -b" public/less/index.less public/css/index.css | ||
$(foreach file, $(filter-out public/less/themes/_base.less, $(wildcard public/less/themes/*)),npx lessc --clean-css="--s0 -b" public/less/themes/$(notdir $(file)) > public/css/theme-$(notdir $(call strip-suffix,$(file))).css;) | ||
$(foreach file, $(wildcard public/css/*),npx postcss --use autoprefixer --autoprefixer.browsers $(BROWSERS) -o $(file) $(file);) | ||
echo "'make generate-stylesheets' is deprecated, please use 'make css'" | ||
$(MAKE) css | ||
|
||
.PHONY: swagger-ui | ||
swagger-ui: | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -136,30 +136,36 @@ You should lint, vet and spell-check with: | |
make vet lint misspell-check | ||
``` | ||
|
||
### Updating the stylesheets | ||
### Updating CSS | ||
|
||
To generate the stylsheets, you will need [Node.js](https://nodejs.org/) at version 8.0 or above. | ||
To generate the CSS, you will need [Node.js](https://nodejs.org/) 8.0 or greater and the build dependencies: | ||
|
||
At present we use [less](http://lesscss.org/) and [postcss](https://postcss.org) to generate our stylesheets. Do | ||
**not** edit the files in `public/css/` directly, as they are generated from | ||
`lessc` from the files in `public/less/`. | ||
```bash | ||
npm install | ||
``` | ||
|
||
If you wish to work on the stylesheets, you will need to install `lessc` the | ||
less compiler and `postcss`. The recommended way to do this is using `npm install`: | ||
At present we use [less](http://lesscss.org/) and [postcss](https://postcss.org) to generate our CSS. Do | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Wouldn't mind if we added mention of what needs to be installed directly on this page in addition to just linking to the pages of those projects. npm install less -g
npm install postcss
npm install clean-css (Or whatever the appropriate commands are if not those) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
If the user did not run There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ugh ok my bad I thought I had run npm install and then didn't. FWIW since I had not, it did fail with this:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Actually this is a bug. The package There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. seems There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Which plugin is missing? Maybe try multiple There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Specifically There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I pushed a fix with multiple Now because of the mentioned bug with There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Pushed the fix with the check for |
||
**not** edit the files in `public/css` directly, as they are generated from `lessc` from the files in `public/less`. | ||
|
||
Edit files in `public/less`, run the linter, regenerate the CSS and commit all changed files: | ||
|
||
```bash | ||
cd "$GOPATH/src/code.gitea.io/gitea" | ||
npm install | ||
make css | ||
``` | ||
|
||
You can then edit the less stylesheets and regenerate the stylesheets using: | ||
### Updating JS | ||
|
||
To run the JavaScript linter you will need [Node.js](https://nodejs.org/) 8.0 or greater and the build dependencies: | ||
|
||
```bash | ||
make generate-stylesheets | ||
npm install | ||
``` | ||
|
||
You should commit both the changes to the css and the less files when making | ||
PRs. | ||
Edit files in `public/js` and run the linter: | ||
|
||
```bash | ||
make js | ||
``` | ||
|
||
### Updating the API | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Happy to remove this rule customization, but there are a few cases of unused function args and marking them with a
_
prefix is somewhat commonplace to indicate they are unused. I think the only other option is to disable that rule.