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

MAINT: refactor webpack.config.js #1035

Merged
merged 5 commits into from Oct 26, 2022
Merged

MAINT: refactor webpack.config.js #1035

merged 5 commits into from Oct 26, 2022

Conversation

12rambau
Copy link
Collaborator

I updated the file to make it more readable:

  • add comments and change variable names to make it more self-explanatory
  • use one-liner when possible to pack functions and variables in blocs
  • drop prettifying as all our strings are too long to be parsed
  • extract the plugin init from module.exports to make it more readable
  • get rid of unused libs (clean-webpack-plugin)

Fix #1030

@12rambau 12rambau marked this pull request as ready for review October 24, 2022 19:19
Copy link
Collaborator

@drammock drammock left a comment

Choose a reason for hiding this comment

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

LGTM, just a couple comments. I don't really understand how webpack works so I'm taking it on faith that you know what you're doing / the test will catch mistakes here.

webpack.config.js Outdated Show resolved Hide resolved
webpack.config.js Outdated Show resolved Hide resolved
webpack.config.js Outdated Show resolved Hide resolved
Copy link
Collaborator

@choldgraf choldgraf left a comment

Choose a reason for hiding this comment

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

This seems pretty good to me in general - the preview docs seem pretty good to me, let's merge it in after addressing @drammock's comments and iterate from there?

12rambau and others added 2 commits October 26, 2022 08:45
Co-authored-by: Daniel McCloy <dan@mccloy.info>
@12rambau
Copy link
Collaborator Author

let's merge it in after addressing @drammock's comments

Done

I don't really understand how webpack works so I'm taking it on faith that you know what you're doing / the test will catch mistakes here.

I'm no expert in webpack either but I'm positive that if something goes wrong (meaning the build of either css or js file fails) then the docs would not render properly in the pre-build. So I think we are on the safe side as everything went ok.

@drammock drammock merged commit 39b6a7f into pydata:main Oct 26, 2022
@12rambau 12rambau deleted the webpack branch October 26, 2022 13:11
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.

CleanWebpackPlugin is never used
3 participants