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

Add webpack option #375

Merged
merged 2 commits into from Mar 11, 2020
Merged

Add webpack option #375

merged 2 commits into from Mar 11, 2020

Conversation

dflupu
Copy link
Contributor

@dflupu dflupu commented Jan 29, 2019

Fixes #143

@sindresorhus sindresorhus force-pushed the master branch 2 times, most recently from 9bbe1bf to 2ab0a69 Compare May 25, 2019 08:31
@sindresorhus sindresorhus changed the title add webpack resolver option Add webpack option Feb 13, 2020
readme.md Outdated Show resolved Hide resolved
readme.md Outdated Show resolved Hide resolved
readme.md Outdated Show resolved Hide resolved
test/options-manager.js Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
lib/options-manager.js Outdated Show resolved Hide resolved
@sindresorhus
Copy link
Member

Sorry for the super slow review. I'm finally getting through all my PRs now.

@dflupu dflupu force-pushed the add-webpack-option branch 2 times, most recently from bad4b2b to f33a474 Compare February 13, 2020 17:26
@sindresorhus
Copy link
Member

You're testing that the config is correctly handled, but I think this needs an integration test too that ensures it actually works in practice in a Webpack project.

lib/options-manager.js Outdated Show resolved Hide resolved
lib/options-manager.js Outdated Show resolved Hide resolved
readme.md Outdated Show resolved Hide resolved
@dflupu
Copy link
Contributor Author

dflupu commented Mar 8, 2020

You're testing that the config is correctly handled, but I think this needs an integration test too that ensures it actually works in practice in a Webpack project.

I'm not sure how that could be done without actually adding Webpack as a dev dependency.

@sindresorhus
Copy link
Member

I'm not sure how that could be done without actually adding Webpack as a dev dependency.

We can add Webpack as a dev dependency.

@sindresorhus sindresorhus merged commit f656ee3 into xojs:master Mar 11, 2020
@sindresorhus
Copy link
Member

Looks good to me now. Thanks for working on it :)

@dflupu
Copy link
Contributor Author

dflupu commented Mar 11, 2020

Thanks for the feedback on this! Felt like I've learned a few things.

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.

Allow configuring towards webpack
2 participants