search

wagtail / sphinx-wagtail-theme

Wagtail's documentation theme for Sphinx
https://sphinx-wagtail-theme.readthedocs.io/
MIT License
30 stars 29 forks source link

Further front-end build cleanup #56

Closed thibaudcolas closed 2 years ago

thibaudcolas commented 3 years ago

Follow-up from #43

tbrlpld commented 3 years ago

Popper is used by bootstrap for certain elements. Not sure we are using any of them.

lb- commented 2 years ago

Should also try to adopt our shared linting packages if not already in use.

PaarthAgarwal commented 2 years ago

Which ones should be present in devDependencies?

lb- commented 2 years ago

Anything that is client facing should be in dependencies. Anything that relates to the building of the project (Webpack) or working with code that gets transformed before the final build (babel, sass) should be Dev.

But check each one gets used also.

"@fortawesome/fontawesome-free": "^5.13.0", non-dev "autocompleter": "^6.0.3", non-dev (I think) "babel-loader": "^8.2.2", DEV "bootstrap": "^4.4.1", non-dev "copy-webpack-plugin": "^8.0.0", DEV "css-loader": "^5.1.2", DEV "file-loader": "^6.2.0", DEV "jquery": "^3.5.1", non-dev "mini-css-extract-plugin": "^1.3.9", DEV "popper.js": "^1.16.1", non-dev (or not needed) "sass": "^1.32.8", DEV "sass-loader": "^11.0.1", DEV "stylelint": "^13.3.3", DEV "underscore": "^1.10.2", non-dev "webpack": "^5.24.4", DEV "webpack-cli": "^4.5.0" DEV

lb- commented 2 years ago

We should also update the package JSON version to match the release version (need to ensure the release instructions includ that).

And add to the list for this issue (or a seperate one) - basic JS unit tests.

PaarthAgarwal commented 2 years ago

We should also update the package JSON version to match the release version (need to ensure the release instructions includ that).

Currently it says "version": "1.0.0". Should it be updated to 2 .0.0 or 3.0.0?

PaarthAgarwal commented 2 years ago

Anything that is client facing should be in dependencies. Anything that relates to the building of the project (Webpack) or working with code that gets transformed before the final build (babel, sass) should be Dev.

But check each one gets used also.

"@fortawesome/fontawesome-free": "^5.13.0", non-dev "autocompleter": "^6.0.3", non-dev (I think) "babel-loader": "^8.2.2", DEV "bootstrap": "^4.4.1", non-dev "copy-webpack-plugin": "^8.0.0", DEV "css-loader": "^5.1.2", DEV "file-loader": "^6.2.0", DEV "jquery": "^3.5.1", non-dev "mini-css-extract-plugin": "^1.3.9", DEV "popper.js": "^1.16.1", non-dev (or not needed) "sass": "^1.32.8", DEV "sass-loader": "^11.0.1", DEV "stylelint": "^13.3.3", DEV "underscore": "^1.10.2", non-dev "webpack": "^5.24.4", DEV "webpack-cli": "^4.5.0" DEV

I think we should keep popper.js. Maybe we are not using it right now but it is possible that one day we'll use something from bootstrap that requires popper. We can remove underscore though as Thibaud suggested.

lb- commented 2 years ago

I would opt for removing popper personally, we can always add it if we need it.

lb- commented 2 years ago

Actually - ignore that package version thing, that is probably just unhelpful and creates confusion.

PaarthAgarwal commented 2 years ago

I would opt for removing popper personally, we can always add it if we need it.

Sure

lb- commented 2 years ago

Last three items have been split to their own issues.

Rest has been done, thanks everyone!