add make targets for js and css, add js linter #6952

silverwind · 2019-05-14T22:48:47Z

add make js, deprecating make javascripts
add make css, deprecating make generate-stylesheets and make stylesheets-check
changed the unclean css check to only run on CI
add JS linting via eslint with basic configuration and fixed discovered issues
changed autoprefixer to use official postcss-cli avoiding the need to loop in the makefile
moved browserslist to package.json so other future tools can use it too
update documentation for new make targets and added JS section

- add `make js`, deprecating `make javascripts` - add `make css`, deprecating `make generate-stylesheets` and `make stylesheets-check` - changed the unclean css check to only run on CI - add JS linting via eslint with basic configuration and fixed discovered issues - changed autoprefixer to use official `postcss-cli` avoiding the need to loop in the makefile - moved browserslist to package.json so other future tools can use it too. - update documentation for new make targets and added JS section

silverwind · 2019-05-14T22:50:46Z

.eslintrc

+  hljs: false
+
+rules:
+  no-unused-vars: [error, {args: all, argsIgnorePattern: ^_, varsIgnorePattern: ^_, ignoreRestSiblings: true}]


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.

silverwind · 2019-05-14T23:02:31Z

Makefile

 	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);)
+	npx postcss --use autoprefixer --no-map --replace public/css/*


Had some weird issue with the foreach on one machine so I switched to the official postcss cli which can process multiple files (and which is also more powerful).

codecov-io · 2019-05-14T23:16:56Z

Codecov Report

Merging #6952 into master will decrease coverage by <.01%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #6952      +/-   ##
==========================================
- Coverage   41.47%   41.47%   -0.01%     
==========================================
  Files         441      441              
  Lines       59662    59662              
==========================================
- Hits        24745    24744       -1     
- Misses      31694    31695       +1     
  Partials     3223     3223

Impacted Files	Coverage Δ
modules/log/colors_router.go	`83.33% <0%> (-4.17%)`	⬇️

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 775a5a5...9cee9f8. Read the comment docs.

silverwind · 2019-05-15T15:17:53Z

Two small fixups pushed. The second one eliminates the eslint-disable comments by moving the functions used in HTML to exported.

mrsdizzie · 2019-05-15T15:45:50Z

docs/content/doc/advanced/hacking-on-gitea.en-us.md


-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


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)

npx is used, so the user won't need to install anything globally.

npm install takes care of it all. It installs bin scripts into node_modules/.bin which npx then calls directly.

If the user did not run npm install it will on-demand fetch the binary from the registry, so the commands should work in all cases as long as the user has a working internet connection.

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:

[mrsdizzie@mbp ~/.go/src/code.gitea.io/gitea (pr-6952-1557933284) ] > make css npx lesshint public/less/ npx: installed 178 in 5.333s npx lessc --clean-css="--s0 -b" public/less/index.less public/css/index.css npm ERR! code E404 npm ERR! 404 Not Found - GET https://registry.npmjs.org/lessc - Not found npm ERR! 404 npm ERR! 404 'lessc@latest' is not in the npm registry. npm ERR! 404 You should bug the author to publish it (or use the name yourself!) npm ERR! 404 npm ERR! 404 Note that you can also install from a npm ERR! 404 tarball, folder, http url, or git url

Actually this is a bug. The package less provides binary lessc, so we should actually use npx less. Investigating possible fixes.

seems npx -p less lessc --clean-css... didn't work without npm i due to plugin for lessc not being installed.

Which plugin is missing? Maybe try multiple -p arguments.

Specifically less-plugin-clean-css. I'm going to pull down the branch and attempt multiple -p args.

I pushed a fix with multiple -p, was also necessary for postcss.

Now because of the mentioned bug with -p, modules are installed every execution even if node_modules is present. I'm thinking maybe I should just re-add the check for presence of node_modules and if its there, assume bin scripts work via regular npx calls.

Pushed the fix with the check for node_modules. I think this is the best compromise as the user only has to install modules once and subsequent runs of make css and make js will be fast that way and we can be sure that all plugins and stuff will be there as well.

This reverts commit 119b725.

* add make targets for js,css, add javascript linter - add `make js`, deprecating `make javascripts` - add `make css`, deprecating `make generate-stylesheets` and `make stylesheets-check` - changed the unclean css check to only run on CI - add JS linting via eslint with basic configuration and fixed discovered issues - changed autoprefixer to use official `postcss-cli` avoiding the need to loop in the makefile - moved browserslist to package.json so other future tools can use it too. - update documentation for new make targets and added JS section * fix indentation * move functions used in html to 'exported' list * Run lessc binary without having to install anything to node_modules * use relative paths to node bin scripts, removing npx * Revert "use relative paths to node bin scripts, removing npx" This reverts commit 119b725. * fix lessc and postcss plugins * check for node_modules and use actual bin names

silverwind commented May 14, 2019

View reviewed changes

GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label May 14, 2019

silverwind commented May 14, 2019

View reviewed changes

techknowlogick added the topic/ui Change the appearance of the Gitea UI label May 15, 2019

techknowlogick added this to the 1.9.0 milestone May 15, 2019

lafriks added the topic/build PR changes how Gitea is built, i.e. regarding Docker or the Makefile label May 15, 2019

lafriks approved these changes May 15, 2019

View reviewed changes

GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels May 15, 2019

mrsdizzie approved these changes May 15, 2019

View reviewed changes

GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels May 15, 2019

silverwind added 2 commits May 15, 2019 17:11

fix indentation

5b7f59a

move functions used in html to 'exported' list

4bd52d7

Merge branch 'master' into make-targets

0679ef0

mrsdizzie reviewed May 15, 2019

View reviewed changes

techknowlogick and others added 7 commits May 15, 2019 11:54

Merge branch 'master' into make-targets

77b954b

Run lessc binary without having to install anything to node_modules

3ebbfb1

use relative paths to node bin scripts, removing npx

119b725

Revert "use relative paths to node bin scripts, removing npx"

eb7ecaf

This reverts commit 119b725.

fix lessc and postcss plugins

8d9d179

check for node_modules and use actual bin names

43e3318

Merge branch 'master' into make-targets

9cee9f8

lafriks merged commit d9dcd09 into go-gitea:master May 16, 2019

lafriks removed the topic/ui Change the appearance of the Gitea UI label May 16, 2019

lunny mentioned this pull request May 16, 2019

make css generate a different package-lock.json on master #6967

Closed

go-gitea locked and limited conversation to collaborators Nov 24, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add make targets for js and css, add js linter #6952

add make targets for js and css, add js linter #6952

silverwind commented May 14, 2019

silverwind May 14, 2019

silverwind May 14, 2019

codecov-io commented May 14, 2019 •

edited

silverwind commented May 15, 2019 •

edited

mrsdizzie May 15, 2019

techknowlogick May 15, 2019

silverwind May 15, 2019 •

edited

mrsdizzie May 15, 2019

silverwind May 15, 2019

techknowlogick May 15, 2019

silverwind May 15, 2019

techknowlogick May 15, 2019

silverwind May 15, 2019

silverwind May 15, 2019

add make targets for js and css, add js linter #6952

add make targets for js and css, add js linter #6952

Conversation

silverwind commented May 14, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov-io commented May 14, 2019 • edited

Codecov Report

silverwind commented May 15, 2019 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

silverwind May 15, 2019 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov-io commented May 14, 2019 •

edited

silverwind commented May 15, 2019 •

edited

silverwind May 15, 2019 •

edited