-
-
Notifications
You must be signed in to change notification settings - Fork 5.6k
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
Types for pipeline operator #9122
Types for pipeline operator #9122
Conversation
Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/9588/ |
@danez @nicolo-ribaudo @xtuc: can you lend an ear here? This is a follow up for #8289. |
}, | ||
}); | ||
|
||
defineType("PipelinePrimaryTopicReference", {}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that this should still be an expression, since it can safely* moved around.
* It's safe only if it is in the correct context, like it would be for any other identifier reference.
I think my last change finishes this PR. I am marking this as WIP just so that I have chance to clean up the commit history, though. Please let me know if you need anything else. |
@thiagoarrais Pretty sure babel team squash/merges so the history should be ok as-is. |
48b880d
to
c7f816b
Compare
Yup. I just want to be nitpicky and remove my temp commits from history. :-) |
This is ready to be merged now. |
Yes, we usually squash-merge (unless multiple authors worked on the same PR).
Lets just wait for another ✔️ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks.
Was surprised that the generated files are committed, but seems I initially did that a while ago 😆So should be okay.
This is the first of two PRs for implementing the smart pipeline transform. It concentrates on the types and generator code associated with the proposal. As soon as this gets merged, I plan on following up with a PR for the transform itself.
It adds three new types that will be needed for writing the visitor for the transform. I've tried to follow existing conventions. But this is my first PR on this area of the code. So please let me know if anything is out of place. I particularly think this needs an accompanying documentation PR, but I'm not really sure where to make the changes.