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.

Sign up for GitHub

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

added examples for transforms. [skip ci] #5393

Merged
merged 4 commits into from Mar 14, 2017
Merged

added examples for transforms. [skip ci] #5393

merged 4 commits into from Mar 14, 2017

Conversation

nitin42
Copy link
Contributor

@nitin42 nitin42 commented Feb 28, 2017

Q A
Patch: Bug Fix? no
Major: Breaking Change? no
Minor: New Feature? no
Deprecations? no
Spec Compliancy? no
Tests Added/Pass? no
Fixed Tickets Fixes #1, Fixes #2
License MIT
Doc PR yes
Dependency Changes no

Added examples for the remaining plugins !

@mention-bot
Copy link

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

@existentialism existentialism added the PR: Docs 📝 A type of pull request used for our changelog categories label Feb 28, 2017
@nitin42 nitin42 changed the title added examples for transforms. [skip ci] added examples for transforms. [skip ci] Feb 28, 2017
existentialism
existentialism reviewed Feb 28, 2017
View reviewed changes
packages/babel-plugin-transform-es2015-sticky-regex/README.md Outdated
## Examples

**In**
```javascript
Copy link
Member

Choose a reason for hiding this comment

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

Since this is included here, we can close #5391

packages/babel-plugin-transform-es2015-block-scoped-functions/README.md Outdated
## Examples

**In**
```javascript
Copy link
Member

Choose a reason for hiding this comment

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

Can we add new lines before code blocks (here and in all the other additions below)

packages/babel-plugin-transform-es2015-block-scoping/README.md Outdated

**In**
```javascript
let msg = "Hello World!";
Copy link
Member

Choose a reason for hiding this comment

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

Not sure this is the best example for block scoping?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Closures ! Right?

packages/babel-plugin-transform-es2015-sticky-regex/README.md Outdated

**Out**
```javascript
"use strict";
Copy link
Member

Choose a reason for hiding this comment

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

Let's drop the "use strict" as it's not really part of the transform

@nitin42
Copy link
Contributor Author

nitin42 commented Feb 28, 2017 •
edited

Sorry! I made an error while adding the skip ci in the commit.

@existentialism existentialism mentioned this pull request Feb 28, 2017
Closed
@xtuc xtuc mentioned this pull request Mar 1, 2017
Closed
14 tasks
xtuc
xtuc reviewed Mar 2, 2017
View reviewed changes
packages/babel-plugin-transform-es2015-block-scoping/README.md Outdated
for (var i = 1; i <= 10; i++) {
_loop(i);
}
```
Copy link
Member

Choose a reason for hiding this comment

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

Could we also add a more simple example? Like:

in

{
  let a = 3
}

let a = 3

out

{
  var _a = 3;
}

var a = 3;

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Why not the current one ? Something ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I know its not casual !

Copy link
Member

Choose a reason for hiding this comment

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

The current one is good but to complex IMO. My example above shows the basic transformation.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So should I replace it with this example ?

Copy link
Member

Choose a reason for hiding this comment

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

What do you mean with the rest? The rest of your PR?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes. The remaining examples for the plugins.

Copy link
Member

Choose a reason for hiding this comment

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

The rest looks good to me

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Gr8!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

had to merge first!

@aaronang
Copy link
Member

aaronang commented Mar 4, 2017 •
edited

In the future, I would recommend you to work in a separate branch other than 7.0. This allows you to keep your 7.0 branch up-to-date with the upstream (babel:7.0) while not affecting your pull requests where you compare your nitin42:feature-branch with babel:7.0.

@nitin42
Copy link
Contributor Author

nitin42 commented Mar 4, 2017

Ok ! 😄

@nitin42 nitin42 mentioned this pull request Mar 6, 2017
Closed
@xtuc
Copy link
Member

xtuc commented Mar 11, 2017 •
edited

Looks fine for me. Could you please rebase with 7.0 branch?

@nitin42
Copy link
Contributor Author

nitin42 commented Mar 14, 2017 •
edited

there were some merge conflicts and the commits were not squashed.

@babel-bot
Copy link
Collaborator

Hey @nitin42! It looks like one or more of your builds have failed. I've copied the relevant info below to save you some time.

@codecov
Copy link

codecov bot commented Mar 14, 2017 •
edited

Codecov Report

Merging #5393 into 7.0 will decrease coverage by 0.03%.
The diff coverage is 97.67%.

@@            Coverage Diff             @@
##              7.0    #5393      +/-   ##
==========================================
- Coverage   85.38%   85.34%   -0.04%     
==========================================
  Files         204      203       -1     
  Lines        9529     9524       -5     
  Branches     2705     2697       -8     
==========================================
- Hits         8136     8128       -8     
- Misses        906      908       +2     
- Partials      487      488       +1
Impacted Files Coverage Δ
packages/babel-generator/src/printer.js 98.4% <100%> (ø) ✅
packages/babel-generator/src/index.js 88% <100%> (ø) ✅
packages/babel-generator/src/node/parentheses.js 97% <97.36%> (-0.03%) ❌
packages/babel-core/src/helpers/resolve.js 92.3% <0%> (-7.7%) ❌
.../transformation/file/options/build-config-chain.js 89.87% <0%> (-4.01%) ❌
packages/babel-traverse/src/path/context.js 85.34% <0%> (-0.87%) ❌
...kages/babel-core/src/transformation/file/logger.js 46.15% <0%> (-0.52%) ❌
packages/babel-traverse/src/path/replacement.js 73.21% <0%> (-0.24%) ❌
...gin-transform-es2015-modules-commonjs/src/index.js 94.44% <0%> (-0.06%) ❌
... and 8 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 0bc802c...2b07e31. Read the comment docs.

@existentialism existentialism merged commit 8c1cd17 into babel:7.0 Mar 14, 2017
@lock lock bot added the outdated A closed issue/PR that is archived due to age. Recommended to make a new issue 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.
Reviewers

@existentialism existentialism existentialism left review comments

@xtuc xtuc xtuc left review comments

Assignees
No one assigned
Labels
outdated A closed issue/PR that is archived due to age. Recommended to make a new issue PR: Docs 📝 A type of pull request used for our changelog categories
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@nitin42 @mention-bot @aaronang @xtuc @babel-bot @existentialism