Skip to content

refactor(estree): re-order serializer code for ImportMeta and NewTarget#24776

Merged
graphite-app[bot] merged 1 commit into
mainfrom
om/07-21-refactor_estree_re-order_serializer_code_for_importmeta_and_newtarget_
Jul 22, 2026
Merged

refactor(estree): re-order serializer code for ImportMeta and NewTarget#24776
graphite-app[bot] merged 1 commit into
mainfrom
om/07-21-refactor_estree_re-order_serializer_code_for_importmeta_and_newtarget_

Conversation

@overlookmotel

@overlookmotel overlookmotel commented Jul 21, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

Follow-on after #24557. Pure refactor.

Personally, I think it's clearer to have helper function below the main code, so it reads top-to-bottom.

It's a matter of taste, but it's consistent with other serializers (since it happens to be my taste, and I wrote most of the serializers).

@github-actions github-actions Bot added the A-ast Area - AST label Jul 21, 2026
@overlookmotel overlookmotel mentioned this pull request Jul 21, 2026
Merged
@overlookmotel Graphite App

overlookmotel commented Jul 21, 2026 •
edited
Loading

Copy link
Copy Markdown
Member Author

How to use the Graphite Merge Queue

Add either label to this PR to merge it via the merge queue:

  • 0-merge - adds this PR to the back of the merge queue
  • hotfix - for urgent changes, fast-track this PR to the front of 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.

This was referenced Jul 21, 2026
Merged
Merged
@overlookmotel
overlookmotel force-pushed the om/07-21-refactor_estree_re-order_serializer_code_for_importmeta_and_newtarget_ branch from f370dd0 to 94c1550 Compare July 21, 2026 19:47
@overlookmotel
overlookmotel force-pushed the om/07-21-fix_estree_handle_empty_spans_serializing_importmeta_and_newtarget_ branch from c4284d7 to 35ae343 Compare July 21, 2026 19:47
@codspeed-hq

codspeed-hq Bot commented Jul 21, 2026 •
edited
Loading

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 67 untouched benchmarks
⏩ 9 skipped benchmarks1

Comparing om/07-21-refactor_estree_re-order_serializer_code_for_importmeta_and_newtarget_ (d81abc3) with om/07-21-fix_estree_handle_empty_spans_serializing_importmeta_and_newtarget_ (6ab74a7)

Open in CodSpeed

Footnotes

  1. 9 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩

@overlookmotel
overlookmotel marked this pull request as ready for review July 21, 2026 19:49
Copilot AI review requested due to automatic review settings July 21, 2026 19:49
Copilot AI reviewed Jul 21, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Refactors the ESTree serializer implementation for ImportMeta and NewTarget meta-properties by re-ordering a local helper function to improve top-to-bottom readability within the serializer module.

Changes:

  • Moved serialize_meta_property_identifier below the ImportMeta* and NewTarget* ESTree impls without changing behavior.

@overlookmotel
overlookmotel requested a review from camc314 July 21, 2026 19:50
@graphite-app
graphite-app Bot force-pushed the om/07-21-fix_estree_handle_empty_spans_serializing_importmeta_and_newtarget_ branch 2 times, most recently from a18c4bc to fc31e79 Compare July 21, 2026 19:54
@graphite-app
graphite-app Bot force-pushed the om/07-21-refactor_estree_re-order_serializer_code_for_importmeta_and_newtarget_ branch from 94c1550 to 1a76b4c Compare July 21, 2026 19:54
@overlookmotel
overlookmotel force-pushed the om/07-21-refactor_estree_re-order_serializer_code_for_importmeta_and_newtarget_ branch from 1a76b4c to eff8b50 Compare July 21, 2026 19:57
@overlookmotel
overlookmotel force-pushed the om/07-21-fix_estree_handle_empty_spans_serializing_importmeta_and_newtarget_ branch 2 times, most recently from 359519f to 6ab74a7 Compare July 21, 2026 20:09
@overlookmotel
overlookmotel force-pushed the om/07-21-refactor_estree_re-order_serializer_code_for_importmeta_and_newtarget_ branch from eff8b50 to d81abc3 Compare July 21, 2026 20:09
@camc314 camc314 added the 0-merge Merge with Graphite Merge Queue label Jul 22, 2026
@camc314 Graphite App

camc314 commented Jul 22, 2026 •
edited by graphite-app Bot
Loading

Copy link
Copy Markdown
Contributor

Merge activity

  • Jul 22, 8:04 AM UTC: The merge label '0-merge' was detected. This PR will be added to the Graphite merge queue once it meets the requirements.
  • Jul 22, 8:04 AM UTC: The merge label '0-merge' was detected. This PR will be added to the Graphite merge queue once it meets the requirements.
  • Jul 22, 8:06 AM UTC: camc314 added this pull request to the Graphite merge queue.
  • Jul 22, 8:17 AM UTC: Merged by the Graphite merge queue.

@graphite-app
graphite-app Bot changed the base branch from om/07-21-fix_estree_handle_empty_spans_serializing_importmeta_and_newtarget_ to graphite-base/24776 July 22, 2026 08:05
@graphite-app
graphite-app Bot changed the base branch from graphite-base/24776 to main July 22, 2026 08:11
@github-actions github-actions Bot added A-linter Area - Linter A-parser Area - Parser A-cli Area - CLI A-linter-plugins Area - Linter JS plugins labels Jul 22, 2026
@overlookmotel
refactor(estree): re-order serializer code for ImportMeta and `NewT…
d68e9fd 
…arget` (#24776)

Follow-on after #24557. Pure refactor.

Personally, I think it's clearer to have helper function below the main code, so it reads top-to-bottom.

It's a matter of taste, but it's consistent with other serializers (since it happens to be my taste, and I wrote most of the serializers).
@graphite-app
graphite-app Bot force-pushed the om/07-21-refactor_estree_re-order_serializer_code_for_importmeta_and_newtarget_ branch from d81abc3 to d68e9fd Compare July 22, 2026 08:11
@graphite-app
graphite-app Bot merged commit d68e9fd into main Jul 22, 2026
30 checks passed
@graphite-app graphite-app Bot removed the 0-merge Merge with Graphite Merge Queue label Jul 22, 2026
@graphite-app
graphite-app Bot deleted the om/07-21-refactor_estree_re-order_serializer_code_for_importmeta_and_newtarget_ branch July 22, 2026 08:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments
@camc314 camc314 camc314 approved these changes

Assignees

@camc314 camc314

Labels

A-ast Area - AST A-cli Area - CLI A-linter Area - Linter A-linter-plugins Area - Linter JS plugins A-parser Area - Parser

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@overlookmotel @camc314