Skip to content

improvement(ast_codegen): better output formatting.#3932

Merged
graphite-app[bot] merged 1 commit intomainfrom
06-26-improvement_ast_codegen_better_output_formatting
Jun 29, 2024
Merged

improvement(ast_codegen): better output formatting.#3932
graphite-app[bot] merged 1 commit intomainfrom
06-26-improvement_ast_codegen_better_output_formatting

Conversation

@rzvxa
Copy link
Contributor

@rzvxa rzvxa commented Jun 26, 2024

We use prettyplease for formatting our generated code, Which has a lot of advantages over rustfmt, But sometimes it conflicts with our cargo fmt config,
Since it isn't configurable I just added a cargo fmt step after the generation process to prevent conflicts.
We might be able to not rely on prettyplease at all, But We have to test that one after we have most of our generated code, rustfmt usually bails on nested calls and macros so it might not work as our only solution. In contrast to that prettyplease is used in both bindgen and cargo-expand and is created for this exact reason(formatting generated code).

Copy link
Contributor Author

rzvxa commented Jun 26, 2024

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @rzvxa and the rest of your teammates on Graphite Graphite

@graphite-app
Copy link
Contributor

graphite-app bot commented Jun 26, 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.

@codspeed-hq
Copy link

codspeed-hq bot commented Jun 26, 2024

CodSpeed Performance Report

Merging #3932 will not alter performance

Comparing 06-26-improvement_ast_codegen_better_output_formatting (71c07c5) with main (7b38bde)

Summary

✅ 28 untouched benchmarks

@rzvxa rzvxa marked this pull request as ready for review June 26, 2024 17:27
@Boshen Boshen added the 0-merge Merge with Graphite Merge Queue label Jun 29, 2024
@graphite-app
Copy link
Contributor

graphite-app bot commented Jun 29, 2024

Merge activity

  • Jun 29, 1:36 AM EDT: The merge label 'merge' was detected. This PR will be added to the Graphite merge queue once it meets the requirements.
  • Jun 29, 1:36 AM EDT: Boshen added this pull request to the Graphite merge queue.
  • Jun 29, 1:41 AM EDT: Boshen merged this pull request with the Graphite merge queue.

We use prettyplease for formatting our generated code, Which has a lot of advantages over rustfmt, But sometimes it conflicts with our cargo fmt config,
Since it isn't configurable I just added a `cargo fmt` step after the generation process to prevent conflicts.
We might be able to not rely on prettyplease at all, But We have to test that one after we have most of our generated code, rustfmt usually bails on nested calls and macros so it might not work as our only solution. In contrast to that prettyplease is used in both bindgen and cargo-expand and is created for this exact reason(formatting generated code).
@Boshen Boshen force-pushed the 06-26-improvement_ast_codegen_better_output_formatting branch from 0bd52d0 to 71c07c5 Compare June 29, 2024 05:37
@graphite-app graphite-app bot merged commit 71c07c5 into main Jun 29, 2024
@graphite-app graphite-app bot deleted the 06-26-improvement_ast_codegen_better_output_formatting branch June 29, 2024 05: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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants