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

[7.0] Drop flowUsesCommas option from babel-generator #5123

merged 1 commit into from Jan 20, 2017


Copy link

@cnguy cnguy commented Jan 14, 2017

Patch: Bug Fix? no
Major: Breaking Change? no
Minor: New Feature? no
Deprecations? yes?
Spec Compliancy? yes?
Tests Added/Pass? yes
Fixed Tickets #5120
License MIT
Doc PR no
Dependency Changes no
  • remove the option for flowUsesCommas in the readme, code, tests
  • just make the default commas (no more semi if its the enforced style at fb already anyway)

In addition to the above, I also deleted packages/babel-generator/test/fixtures/flowUsesCommas/options.json because it only had one option (flowCommaSeparator).

Copy link

@mention-bot mention-bot commented Jan 14, 2017

@ChauTNguyen, thanks for your PR! By analyzing the history of the files in this pull request, we identified @sampepose, @bhosmer and @DrewML to be potential reviewers.

@cnguy cnguy changed the title Drop flowUsesCommas option from babel-generator #[5120]( Drop flowUsesCommas option from babel-generator Jan 14, 2017
Copy link

@codecov-io codecov-io commented Jan 15, 2017

Current coverage is 89.20% (diff: 100%)

Merging #5123 into 7.0 will increase coverage by <.01%

@@                7.0      #5123   diff @@
  Files           203        203          
  Lines          9819       9820     +1   
  Methods        1071       1071          
  Messages          0          0          
  Branches       2615       2615          
+ Hits           8759       8760     +1   
  Misses         1060       1060          
  Partials          0          0          

Powered by Codecov. Last update 672adba...a0f5377

Copy link

@loganfsmyth loganfsmyth commented Jan 15, 2017

Seems like a breaking change to me, so probably a good candidate for 7.x

Copy link

@loganfsmyth loganfsmyth commented Jan 15, 2017

Oh this is for #5120 so yeah 7.x. Going to move your PR to be based on the 7.0 branch.

@loganfsmyth loganfsmyth changed the base branch from master to 7.0 Jan 15, 2017
@loganfsmyth loganfsmyth changed the title Drop flowUsesCommas option from babel-generator [7.0] Drop flowUsesCommas option from babel-generator Jan 15, 2017
@danez danez added this to the Babel 7 milestone Jan 15, 2017
@hzoo hzoo merged commit d710e6d into babel:7.0 Jan 20, 2017
3 checks passed
3 checks passed
codecov/patch 100% of diff hit (target 89.20%)
codecov/project 89.20% (+<.01%) compared to 672adba
continuous-integration/travis-ci/pr The Travis CI build passed
Copy link

@hzoo hzoo commented Jan 20, 2017

Nice work @ChauTNguyen!! (merged to 7.0 branch)

@hzoo hzoo mentioned this pull request Jan 20, 2017
0 of 2 tasks complete
@cnguy cnguy deleted the cnguy:flow-default-commas branch Jan 20, 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.
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

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