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

Jump to bottom

feat: add --config-name flag #1753

Merged
merged 2 commits into from Aug 19, 2020
Merged

feat: add --config-name flag #1753

merged 2 commits into from Aug 19, 2020

Conversation

snitin315
Copy link
Member

@snitin315 snitin315 commented Aug 18, 2020 •
edited

What kind of change does this PR introduce?
Feature

Did you add tests for your changes?
Yes
If relevant, did you update the documentation?
Yes
Summary
Addition of --config-name flag.

Does this PR introduce a breaking change?
No

Other information
Fixes #1657

@anshumanv
Copy link
Member

I think it makes sense to add multiple config support using flags first.

@snitin315
Copy link
Member Author

snitin315 commented Aug 18, 2020 •
edited

--config-name works with a single config file exporting an array of configurations.

https://webpack.js.org/configuration/configuration-types/#exporting-multiple-configurations

@snitin315 snitin315 marked this pull request as ready for review August 18, 2020 08:10
@snitin315 snitin315 requested a review from a team as a code owner August 18, 2020 08:10
anshumanv
anshumanv reviewed Aug 18, 2020
View reviewed changes
packages/webpack-cli/lib/groups/ConfigGroup.js
@@ -117,6 +118,14 @@ class ConfigGroup extends GroupHelper {
const newOptions = configOptions(formattedEnv, argv);
// When config function returns a promise, resolve it, if not it's resolved by default
newOptionsObject['options'] = await Promise.resolve(newOptions);
} else if (Array.isArray(configOptions) && this.args.configName) {
Copy link
Member

Choose a reason for hiding this comment

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

Add a comment when this block is executed, can give example, looks good otherwise

@snitin315
Copy link
Member Author

This commitlint check is unstable these days. Can we remove it as a required check?

@alexander-akait
Copy link
Member

@snitin315 maybe we should update commitlint action, need look at this

@anshumanv
Copy link
Member

@snitin315 maybe we should update commitlint action, need look at this

I think it's setup externally, not defined in the workflow file. @evenstensberg do you know how it's setup?

@jamesgeorge007
Copy link
Member

It seems we're relying on commitlint-bot which is not maintained anymore, it is advised to use semantic-pull-requests instead.

@alexander-akait alexander-akait merged commit d3ed19a into next Aug 19, 2020
@alexander-akait alexander-akait deleted the feat/config-name branch August 19, 2020 10:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@alexander-akait alexander-akait alexander-akait approved these changes

@evenstensberg evenstensberg evenstensberg left review comments

@rishabh3112 rishabh3112 rishabh3112 approved these changes

@shivaylamba shivaylamba shivaylamba approved these changes

@anshumanv anshumanv anshumanv approved these changes

@jamesgeorge007 jamesgeorge007 jamesgeorge007 approved these changes

Assignees
No one assigned
Labels
PR: reviewed-approved
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Support setting the config name for multiple configs
8 participants
@snitin315 @anshumanv @alexander-akait @jamesgeorge007 @rishabh3112 @evenstensberg @shivaylamba @webpack-bot