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

Feature: Passthru metadata #398

Merged
merged 1 commit into from Mar 5, 2017
Merged

Feature: Passthru metadata #398

merged 1 commit into from Mar 5, 2017

Conversation

Ognian
Copy link
Contributor

@Ognian Ognian commented Feb 28, 2017 •
edited

Please Read the CONTRIBUTING Guidelines
In particular the portion on Commit Message Formatting

Please check if the PR fulfills these requirements

  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)

What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)

  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Other... Please describe:

What is the current behavior? (You can also link to an open issue here)

What is the new behavior?

Does this PR introduce a breaking change?

  • Yes
  • No

If this PR contains a breaking change, please describe the following...

  • Impact:
  • Migration path for existing applications:
  • Github Issue(s) this is regarding:

Other information:

@codecov
Copy link

codecov bot commented Feb 28, 2017 •
edited

Codecov Report

Merging #398 into master will increase coverage by 0.49%.
The diff coverage is 90.9%.

@@            Coverage Diff             @@
##           master     #398      +/-   ##
==========================================
+ Coverage   83.03%   83.52%   +0.49%     
==========================================
  Files           6        6              
  Lines         165      176      +11     
  Branches       38       39       +1     
==========================================
+ Hits          137      147      +10     
  Misses         12       12              
- Partials       16       17       +1
Impacted Files Coverage Δ
src/index.js 86.04% <90.9%> (+0.71%) ✅

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 2fbcc01...edebae7. Read the comment docs.

@Ognian Ognian mentioned this pull request Feb 28, 2017
Closed
11 tasks
@Ognian Ognian force-pushed the pass_metadata_v2 branch 2 times, most recently from 532fb02 to 9f94dee Compare March 1, 2017 11:48
@Ognian Ognian changed the title - added metadata passing from babel to webpack Feature: Passthru metadata Mar 1, 2017
@Ognian
Copy link
Contributor Author

Ognian commented Mar 1, 2017

@danez I think that this pr can now be considered to be merged. Please answer in a timely manner, since now I'm up to date again and can quickly implement some changes if desired. You should close #329 too (I can't do this since I'm not the owner)
Thanks
Ognian

danez
danez suggested changes Mar 4, 2017
View reviewed changes
Copy link
Member

@danez danez left a comment •
edited

Choose a reason for hiding this comment

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

Okay after more thinking about it I think there is no other way to achieve this.
I made some comments about some minor changes. Can you address them?

src/index.js Outdated
@@ -134,10 +146,16 @@ module.exports = function(source, inputSourceMap) {
transform: transpile,
}, function(err, result) {
if (err) { return callback(err); }
metadataSubscribers.map(function (s) {
Copy link
Member

@danez danez Mar 4, 2017 •
edited

Choose a reason for hiding this comment

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

No need for .map(). Can use .forEach()

src/index.js Outdated
return callback(null, result.code, result.map);
});
}

const result = transpile(source, options);
metadataSubscribers.map(function (s) {
Copy link
Member

Choose a reason for hiding this comment

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

Same here

.gitignore Outdated
@@ -4,3 +4,4 @@ lib
node_modules
npm-debug.log
test/output
.idea
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 please revert this. You should ignore this folder globally on your machine.
https://help.github.com/articles/ignoring-files/#create-a-global-gitignore

package.json Outdated
@@ -32,6 +32,10 @@
"eslint-plugin-flowtype": "^2.25.0",
"nyc": "^10.0.0",
"rimraf": "^2.4.3",
"react": "^15.1.0",
"react-intl":"^2.1.2",
"babel-plugin-react-intl": "2.1.3",
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 do ^2.1.3 here so we test agains the latest version 2.3.0

@danez danez merged commit 1fbbdf3 into babel:master Mar 5, 2017
@Ognian Ognian deleted the pass_metadata_v2 branch March 5, 2017 13:48
@mbrevda
Copy link

mbrevda commented Mar 5, 2017

Waho 🎉 ! Thanks so much @Ognian! Are there docs/examples anywhere for how to access the metadata now?

@mbrevda mbrevda mentioned this pull request Mar 5, 2017
Closed
danez added a commit that referenced this pull request Mar 6, 2017
@danez
Merge branch 'master' into 7.0
70c8c4a 
* master:
  added metadata passing from babel to webpack (#398)
  document globalOptions (#365)
  Docs: change babel-preset-es2015 to babel-preset-env (#404) [skip ci]

# Conflicts:
#	README.md
#	package.json
#	src/index.js
danez added a commit that referenced this pull request Mar 6, 2017
@danez
Optimize code after merge of #398
ca3318b
danez added a commit that referenced this pull request Mar 6, 2017
@danez
Merge branch 'master' into 7.0
906ab55 
* master:
  6.4.0
  Update CHANGELOG.md for 6.4.0
  Optimize code after merge of #398
  Update yarn.lock
  Sort package.json

# Conflicts:
#	package.json
#	src/index.js
#	yarn.lock
@hzoo hzoo mentioned this pull request Mar 28, 2017
Closed
This was referenced Mar 19, 2018
Closed
Merged
@jameswomack jameswomack mentioned this pull request Jul 9, 2020
Merged
11 tasks
@dependabot dependabot bot mentioned this pull request Mar 16, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@danez danez danez approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@Ognian @mbrevda @danez