Skip to content

Comments

refactor(ast, transformer)!: remove BlockStatement::new methods#6783

Merged
graphite-app[bot] merged 1 commit intomainfrom
10-22-refactor_ast_transformer_remove_blockstatement_new_methods
Oct 23, 2024
Merged

refactor(ast, transformer)!: remove BlockStatement::new methods#6783
graphite-app[bot] merged 1 commit intomainfrom
10-22-refactor_ast_transformer_remove_blockstatement_new_methods

Conversation

@overlookmotel
Copy link
Member

@overlookmotel overlookmotel commented Oct 22, 2024

First of a series of PRs removing new and new_with_scope_id etc methods from AST types. Following #6760, all AST node creation can now go via the AST builder.

This lays groundwork for Node IDs (#5689), as we'll need NodeIds to be generated in AstBuilder, so that all nodes receive an ID.

@graphite-app
Copy link
Contributor

graphite-app bot commented Oct 22, 2024

Your org has enabled the Graphite merge queue for merging into main

Add the label “0-merge” to the PR and Graphite will automatically add it to the merge queue when it’s ready to merge. Or use the label “hotfix” to add to the merge queue as a hot fix.

You must have a Graphite account and log in to Graphite in order to use the merge queue. Sign up using this link.

@github-actions github-actions bot added A-ast Area - AST A-transformer Area - Transformer / Transpiler labels Oct 22, 2024
@github-actions github-actions bot added the C-cleanup Category - technical debt or refactoring. Solution not expected to change behavior label Oct 22, 2024
@overlookmotel overlookmotel changed the title refactor(ast, transformer): remove BlockStatement::new methods refactor(ast, transformer)!: remove BlockStatement::new methods Oct 22, 2024
@overlookmotel overlookmotel marked this pull request as ready for review October 22, 2024 17:05
@overlookmotel overlookmotel requested a review from Boshen October 22, 2024 17:08
@overlookmotel
Copy link
Member Author

overlookmotel commented Oct 22, 2024

@Boshen I imagine something in this stack will break Rolldown. I assume they have access to AstBuilder, so it'll be an easy fix to migrate over to using AstBuilder methods to generate AST nodes?

@codspeed-hq
Copy link

codspeed-hq bot commented Oct 22, 2024

CodSpeed Performance Report

Merging #6783 will not alter performance

Comparing 10-22-refactor_ast_transformer_remove_blockstatement_new_methods (2bee4e2) with main (8f17953)

Summary

✅ 30 untouched benchmarks

@Boshen Boshen added the 0-merge Merge with Graphite Merge Queue label Oct 23, 2024 — with Graphite App
@graphite-app
Copy link
Contributor

graphite-app bot commented Oct 23, 2024

Merge activity

)

First of a series of PRs removing `new` and `new_with_scope_id` etc methods from AST types. Following #6760, all AST node creation can now go via the AST builder.

This lays groundwork for Node IDs (#5689), as we'll need `NodeId`s to be generated in `AstBuilder`, so that all nodes receive an ID.
@Boshen Boshen force-pushed the 10-22-refactor_ast_transformer_remove_blockstatement_new_methods branch from 38530e2 to 2bee4e2 Compare October 23, 2024 03:36
@graphite-app graphite-app bot merged commit 2bee4e2 into main Oct 23, 2024
@graphite-app graphite-app bot deleted the 10-22-refactor_ast_transformer_remove_blockstatement_new_methods branch October 23, 2024 03:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

0-merge Merge with Graphite Merge Queue A-ast Area - AST A-transformer Area - Transformer / Transpiler C-cleanup Category - technical debt or refactoring. Solution not expected to change behavior

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants