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

refactor(var-declarations): remove unnecessary init parameter from insert_var #7668

Conversation

Dunqing
Copy link
Member

@Dunqing Dunqing commented Dec 5, 2024

If we want to pass init, we should use insert_var_with_init

Copy link

graphite-app bot commented Dec 5, 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 the C-cleanup Category - technical debt or refactoring. Solution not expected to change behavior label Dec 5, 2024
@Dunqing Dunqing changed the title refactor(var-declarations): remove unnecessary init parameter from insert_var refactor(var-declarations): remove unnecessary init parameter from insert_var Dec 5, 2024
@Dunqing Dunqing force-pushed the 12-05-refactor_var-declarations_remove_unnecessary_init_parameter_from_insert_var branch from c46aaa4 to a3bd333 Compare December 5, 2024 03:51
@Dunqing Dunqing force-pushed the 12-05-feat_transformer_var-declaration_add_insert_var_with_init_method branch from 1124b35 to 1541969 Compare December 5, 2024 03:57
@Dunqing Dunqing force-pushed the 12-05-refactor_var-declarations_remove_unnecessary_init_parameter_from_insert_var branch from a3bd333 to f289f33 Compare December 5, 2024 03:57
Copy link

codspeed-hq bot commented Dec 5, 2024

CodSpeed Performance Report

Merging #7668 will not alter performance

Comparing 12-05-refactor_var-declarations_remove_unnecessary_init_parameter_from_insert_var (3d593ec) with main (0ca10e2)

Summary

✅ 29 untouched benchmarks

@overlookmotel overlookmotel force-pushed the 12-05-feat_transformer_var-declaration_add_insert_var_with_init_method branch from 1541969 to 0bd5333 Compare December 5, 2024 13:48
@overlookmotel overlookmotel force-pushed the 12-05-refactor_var-declarations_remove_unnecessary_init_parameter_from_insert_var branch from f289f33 to ffe659f Compare December 5, 2024 13:48
Copy link
Contributor

@overlookmotel overlookmotel left a comment

Choose a reason for hiding this comment

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

This is all great. So much boilerplate code removed.

Thanks for splitting it into many small PRs - made it much easier to review.

@overlookmotel overlookmotel changed the base branch from 12-05-feat_transformer_var-declaration_add_insert_var_with_init_method to main December 5, 2024 14:05
@overlookmotel
Copy link
Contributor

Oh bollocks. I screwed up and rebased this on main by accident. Oops! I'll sort it out once the downstack PRs are all merged.

@Dunqing Dunqing force-pushed the 12-05-refactor_var-declarations_remove_unnecessary_init_parameter_from_insert_var branch from ffe659f to 963f0c5 Compare December 5, 2024 14:20
@Dunqing Dunqing changed the base branch from main to 12-05-feat_transformer_var-declaration_add_insert_var_with_init_method December 5, 2024 14:20
@overlookmotel overlookmotel force-pushed the 12-05-feat_transformer_var-declaration_add_insert_var_with_init_method branch from d10496e to eb0ba85 Compare December 5, 2024 14:24
@overlookmotel overlookmotel force-pushed the 12-05-refactor_var-declarations_remove_unnecessary_init_parameter_from_insert_var branch from 963f0c5 to d2bd3fd Compare December 5, 2024 14:24
@overlookmotel overlookmotel added the 0-merge Merge with Graphite Merge Queue label Dec 5, 2024
Copy link
Contributor

overlookmotel commented Dec 5, 2024

Merge activity

  • Dec 5, 9:27 AM EST: The merge label '0-merge' was detected. This PR will be added to the Graphite merge queue once it meets the requirements.
  • Dec 5, 9:33 AM EST: A user added this pull request to the Graphite merge queue.
  • Dec 5, 9:46 AM EST: A user merged this pull request with the Graphite merge queue.

…`insert_var` (#7668)

If we want to pass `init`, we should use `insert_var_with_init`
@overlookmotel overlookmotel force-pushed the 12-05-feat_transformer_var-declaration_add_insert_var_with_init_method branch from eb0ba85 to e8518e9 Compare December 5, 2024 14:38
@overlookmotel overlookmotel force-pushed the 12-05-refactor_var-declarations_remove_unnecessary_init_parameter_from_insert_var branch from d2bd3fd to 3d593ec Compare December 5, 2024 14:39
Base automatically changed from 12-05-feat_transformer_var-declaration_add_insert_var_with_init_method to main December 5, 2024 14:43
@graphite-app graphite-app bot merged commit 3d593ec into main Dec 5, 2024
26 checks passed
@graphite-app graphite-app bot deleted the 12-05-refactor_var-declarations_remove_unnecessary_init_parameter_from_insert_var branch December 5, 2024 14:46
overlookmotel added a commit that referenced this pull request Dec 5, 2024
Follow-on after #7668. Rename `create_var*` methods to `create_uid_var*`.

Previous method name `create_var` might suggest that it creates a binding with the provided name. But actually it creates a UID with name *based on* the name provided.
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-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