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

fix: tagged template incorrect receiver #13395

Merged
merged 6 commits into from Jun 15, 2021

Conversation

sag1v
Copy link
Contributor

@sag1v sag1v commented May 29, 2021

Q                       A
Fixed Issues? Fixes #13366
Patch: Bug Fix? 👍
Major: Breaking Change?
Minor: New Feature?
Tests Added + Pass? 👍
Documentation PR Link
Any Dependency Changes?
License MIT

@babel-bot
Copy link
Collaborator

babel-bot commented May 29, 2021

Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/46894/

@codesandbox-ci
Copy link

codesandbox-ci bot commented May 29, 2021

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 9a0d7ec:

Sandbox Source
babel-repl-custom-plugin Configuration
babel-plugin-multi-config Configuration

@nicolo-ribaudo nicolo-ribaudo added PR: Bug Fix 🐛 A type of pull request used for our changelog categories Spec: Class Fields Spec: Private Methods labels May 29, 2021
@sag1v
Copy link
Contributor Author

sag1v commented May 29, 2021

@nicolo-ribaudo Thanks for the comments.

General question, Can i create a new commit (fixing review) and push to this PR or do i must keep it with a single commit (using commit --amend with force push)?

@jridgewell
Copy link
Member

It's fine to add as many commits as needed to this PR.

@sag1v
Copy link
Contributor Author

sag1v commented May 29, 2021

@nicolo-ribaudo @jridgewell I pushed the new changes per your comments

@sag1v
Copy link
Contributor Author

sag1v commented Jun 1, 2021

@nicolo-ribaudo / @jridgewell Are we done here? :)

Copy link
Member

@nicolo-ribaudo nicolo-ribaudo left a comment

Choose a reason for hiding this comment

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

I left a few more comments about tests; apart from them it looks good 👍

@sag1v sag1v requested a review from jridgewell June 12, 2021 10:54
Copy link
Member

@jridgewell jridgewell left a comment

Choose a reason for hiding this comment

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

Sorry, just a few more comments (that I screwed up). If you can make these changes, run update the tests one last time, this should be good to merge.

@sag1v
Copy link
Contributor Author

sag1v commented Jun 15, 2021

@jridgewell Thanks, done.

Copy link
Member

@existentialism existentialism left a comment

Choose a reason for hiding this comment

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

👍

@JLHwung JLHwung merged commit a3c7497 into babel:main Jun 15, 2021
@github-actions github-actions bot added the outdated A closed issue/PR that is archived due to age. Recommended to make a new issue label Sep 15, 2021
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 15, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
outdated A closed issue/PR that is archived due to age. Recommended to make a new issue PR: Bug Fix 🐛 A type of pull request used for our changelog categories Spec: Class Fields Spec: Private Methods
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: Tagged template with strict private field/method tag has incorrect receiver
6 participants