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

More strictly parse configs and explicitly handle arguments in babel-cli #5463

Merged
merged 4 commits into from Mar 14, 2017

Conversation

@loganfsmyth
Copy link
Member

@loganfsmyth loganfsmyth commented Mar 13, 2017

Q A
Patch: Bug Fix? N
Major: Breaking Change? Y
Minor: New Feature? N
Deprecations?
Spec Compliancy?
Tests Added/Pass?
Fixed Tickets
License MIT
Doc PR
Dependency Changes

Breaking changes:

  • This deletes the 'options' export from babel-core.
  • Babel options that expect arrays must be arrays, where before strings would be split on commas and single values would be converted to arrays. The CLI still supports comma-strings however. Hopefully will effect relatively few people.

The primary goal of this PR is to get rid of our config.js file that listed options in favor of defaults + validation inside the option manager. The primary user-visible change is the required array inputs.

@mention-bot
Copy link

@mention-bot mention-bot commented Mar 13, 2017

@loganfsmyth, thanks for your PR! By analyzing the history of the files in this pull request, we identified @existentialism, @hzoo and @DmitrySoshnikov to be potential reviewers.

@codecov
Copy link

@codecov codecov bot commented Mar 13, 2017

Codecov Report

Merging #5463 into 7.0 will decrease coverage by 0.03%.
The diff coverage is 79.68%.

@@            Coverage Diff            @@
##             7.0    #5463      +/-   ##
=========================================
- Coverage   85.4%   85.37%   -0.04%     
=========================================
  Files        203      201       -2     
  Lines       9525     9522       -3     
  Branches    2703     2700       -3     
=========================================
- Hits        8135     8129       -6     
+ Misses       903      901       -2     
- Partials     487      492       +5
Impacted Files Coverage Δ
packages/babel-core/src/index.js 58.82% <ø> (ø)
packages/babel-cli/src/babel/util.js 61.11% <100%> (+1.65%)
...ckages/babel-core/src/transformation/file/index.js 85% <100%> (+0.24%)
packages/babel-cli/src/babel/dir.js 53.65% <100%> (ø)
packages/babel-cli/src/babel/file.js 75.94% <50%> (ø)
.../src/transformation/file/options/option-manager.js 79.13% <62.5%> (-4.35%)
packages/babel-cli/src/babel/index.js 68.83% <86.48%> (+2.16%)
packages/babel-traverse/src/path/context.js 86.2% <0%> (+0.86%)
packages/babel-traverse/src/visitors.js 86.66% <0%> (+0.95%)
... and 1 more

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 523a41b...5b50b73. Read the comment docs.

"moduleIds",
"moduleId",
"passPerPreset",
// Deprecate top level parserOpts

This comment has been minimized.

@hzoo

hzoo Mar 14, 2017
Member

I think I know what this means now (I believe I wrote this by saying we should remove the other options in this set that would only be options for either the parser or generator.

ex: "comments" vs generatorOpts.comments?

This comment has been minimized.

@loganfsmyth

loganfsmyth Mar 14, 2017
Author Member

Yeah, I'd have to refresh my memory too :P

@hzoo hzoo added this to the Babel 7 milestone Mar 14, 2017
@loganfsmyth loganfsmyth merged commit 2642c2c into babel:7.0 Mar 14, 2017
1 of 3 checks passed
1 of 3 checks passed
codecov/patch 79.68% of diff hit (target 85.4%)
Details
codecov/project 85.37% (-0.04%) compared to c00ffb8
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@loganfsmyth loganfsmyth deleted the loganfsmyth:strict-args branch Mar 14, 2017
@lock lock bot added the outdated label Oct 6, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Oct 6, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.