-
-
Notifications
You must be signed in to change notification settings - Fork 504
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(ast): estree compat AssignmentTargetPropertyIdentifier
#9006
fix(ast): estree compat AssignmentTargetPropertyIdentifier
#9006
Conversation
How to use the Graphite Merge QueueAdd either label to this PR to merge it via the merge queue:
You must have a Graphite account in order to use the merge queue. Sign up using this link. An organization admin has enabled the Graphite Merge Queue in this repository. Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue. This stack of pull requests is managed by Graphite. Learn more about stacking. |
AssignmentTargetPropertyIdentifier
0193c95
to
8a3c346
Compare
CodSpeed Performance ReportMerging #9006 will not alter performanceComparing Summary
|
8a3c346
to
f084e83
Compare
6d7c3c7
to
dd927a6
Compare
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.
Yes, I think there's a simpler way to implement this. Merging this now, and I'll do a follow-up PR.
Merge activity
|
@hi-ogawa By the way, it's been a bit of a struggle to get all these PRs merged, due to inadequacies in Graphite and the merge conflicts in the snapshot file. One thing that would help is if you'd be able to squash all PRs down to a single commit each please. Multiple commits should be no problem, but squashing seems to confuse Graphite less. Also, please feel free to merge your own PRs in this area of work. If it passes more tests, it's good! I can always follow up after they're merged if I want to make tweaks or query anything. |
This one seemed less straight forward than `AssignmentTargetPropertyProperty`, so I split it from #9005. This requires somewhat redundant custom serialization logic, but this is all I came up with. I'm curious to know if there's any good shortcut for this.
f084e83
to
22d93be
Compare
@overlookmotel Understood! Sorry about the conflict and thanks for taking care of them 🙏 I'll try to proceed my own for easier merge. |
This one seemed less straight forward than
AssignmentTargetPropertyProperty
, so I split it from #9005. This requires somewhat redundant custom serialization logic, but this is all I came up with. I'm curious to know if there's any good shortcut for this.