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] Remove the unneeded Pipeline class. #5376

Merged
merged 1 commit into from Feb 27, 2017

Conversation

@loganfsmyth
Copy link
Member

commented Feb 24, 2017

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

I'm trying to remove things from the public API that aren't actually needed for public use. The official APIs for this are already available on the overall babel-core import, there isn't any reason to expose this class too.

I removed pretransform and lint because they don't seem to do anything useful. If we want to keep them, we should expose them like transform and analyze and such instead of using a class.

@mention-bot

This comment has been minimized.

Copy link

commented Feb 24, 2017

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

@codecov

This comment has been minimized.

Copy link

commented Feb 24, 2017

Codecov Report

Merging #5376 into 7.0 will increase coverage by 0.08%.
The diff coverage is 66.66%.

@@            Coverage Diff             @@
##              7.0    #5376      +/-   ##
==========================================
+ Coverage   90.38%   90.47%   +0.08%     
==========================================
  Files         204      204              
  Lines        9891     9876      -15     
  Branches     2695     2694       -1     
==========================================
- Hits         8940     8935       -5     
+ Misses        951      941      -10
Impacted Files Coverage Δ
packages/babel-core/src/index.js 77.77% <ø> (-4.05%)
...ckages/babel-core/src/transformation/file/index.js 90.6% <100%> (-0.04%)
packages/babel-core/src/transformation/pipeline.js 63.15% <63.15%> (+21.77%)

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 87ca615...f3e9201. Read the comment docs.

@xtuc

This comment has been minimized.

Copy link
Member

commented Feb 25, 2017

I think we should mark this as a breaking change since it was part of the public API.
Migration is easy because you just need to use the functions exposed by babel-core instead.


transformFromAst(ast: Object, code: string, opts: Object): BabelFileResult {
ast = normalizeAst(ast);
export function transformFromAst(ast: Object, code: string, opts: Object): BabelFileResult {

This comment has been minimized.

Copy link
@danez

danez Feb 27, 2017

Member

We should create ticket to test transformFromAst

@danez
danez approved these changes Feb 27, 2017
@loganfsmyth loganfsmyth changed the title Remove the unneeded Pipeline class. [7.0] Remove the unneeded Pipeline class. Feb 27, 2017
@loganfsmyth loganfsmyth merged commit 9acae54 into babel:7.0 Feb 27, 2017
2 of 3 checks passed
2 of 3 checks passed
codecov/patch 66.66% of diff hit (target 90.38%)
Details
codecov/project 90.47% (+0.08%) compared to 87ca615
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@loganfsmyth loganfsmyth deleted the loganfsmyth:no-pipeline branch Feb 27, 2017
@bjornharrtell bjornharrtell referenced this pull request Sep 28, 2018
@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
4 participants
You can’t perform that action at this time.