Skip to content

Conversation

@overlookmotel
Copy link
Member

@overlookmotel overlookmotel commented Aug 9, 2024

Follow-on after #4778. ///@@line_break is more verbose, but it's clearer what it does.

@graphite-app
Copy link
Contributor

graphite-app bot commented Aug 9, 2024

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

Add the label “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.

@overlookmotel overlookmotel marked this pull request as ready for review August 9, 2024 10:15
@rzvxa
Copy link
Contributor

rzvxa commented Aug 9, 2024

It is really weird that the line break happens at the end😕 But I guess this one is a bit more clear.

@Dunqing Dunqing changed the title refactor(codegen): replace ///@@ with ///@@line_break refactor(ast_codegen): replace ///@@ with ///@@line_break Aug 9, 2024
@codspeed-hq
Copy link

codspeed-hq bot commented Aug 9, 2024

CodSpeed Performance Report

Merging #4786 will not alter performance

Comparing 08-09-refactor_codegen_replace___with___line_break_ (92777d0) with main (3131187)

Summary

✅ 29 untouched benchmarks

@overlookmotel
Copy link
Member Author

It is really weird that the line break happens at the end😕

How do you mean "at the end"?

@overlookmotel overlookmotel force-pushed the 08-09-refactor_ast_codegen_trim_r_from_start_of_field_names branch from b428568 to 875a9cb Compare August 9, 2024 10:30
@overlookmotel overlookmotel force-pushed the 08-09-refactor_codegen_replace___with___line_break_ branch from ba53404 to 7bbde41 Compare August 9, 2024 10:30
@rzvxa
Copy link
Contributor

rzvxa commented Aug 9, 2024

In the sense that it can't be a free-standing thing, It needs to come before an item so we can't have stuff at the end of a scope.

@rzvxa rzvxa added the 0-merge Merge with Graphite Merge Queue label Aug 9, 2024
@graphite-app
Copy link
Contributor

graphite-app bot commented Aug 9, 2024

Merge activity

  • Aug 9, 6:51 AM EDT: The merge label 'merge' was detected. This PR will be added to the Graphite merge queue once it meets the requirements.
  • Aug 9, 6:51 AM EDT: rzvxa added this pull request to the Graphite merge queue.
  • Aug 9, 6:58 AM EDT: rzvxa merged this pull request with the Graphite merge queue.

Follow-on after #4778. `///@@line_break` is more verbose, but it's clearer what it does.
@rzvxa rzvxa force-pushed the 08-09-refactor_ast_codegen_trim_r_from_start_of_field_names branch from 875a9cb to ec82a79 Compare August 9, 2024 10:53
@rzvxa rzvxa force-pushed the 08-09-refactor_codegen_replace___with___line_break_ branch from 7bbde41 to 92777d0 Compare August 9, 2024 10:53
Base automatically changed from 08-09-refactor_ast_codegen_trim_r_from_start_of_field_names to main August 9, 2024 10:57
@graphite-app graphite-app bot merged commit 92777d0 into main Aug 9, 2024
@graphite-app graphite-app bot deleted the 08-09-refactor_codegen_replace___with___line_break_ branch August 9, 2024 10:58
@overlookmotel
Copy link
Member Author

In the sense that it can't be a free-standing thing, It needs to come before an item so we can't have stuff at the end of a scope.

Ah I see. Yes it is a bit weird. Annoying that there seems to be no way to do something as simple as line breaks without all these odd workarounds.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants