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(prettier): Refactor IR related macros #7491

Merged
merged 1 commit into from
Nov 26, 2024
Merged

Conversation

leaysgur
Copy link
Collaborator

  • Remove format! macro
  • Rename string related macros, &'static: text! and 'a: dynamic_text!
  • Apply wrap! macro instead of manually using enter|leave_node
  • Introduce /ir directory and move Doc, impl Display, DocBuilder, etc.

I'm not yet determined how to, how deep to use macro, but this is first stepping stone... 🥌

Copy link

graphite-app bot commented Nov 26, 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-prettier Area - Prettier C-cleanup Category - technical debt or refactoring. Solution not expected to change behavior labels Nov 26, 2024
@leaysgur
Copy link
Collaborator Author

leaysgur commented Nov 26, 2024

free to make any changes to the oxc_prettier crate.
#5068 (comment)

@Boshen Can I understand this to mean that I can merge PRs without review?

Copy link

codspeed-hq bot commented Nov 26, 2024

CodSpeed Performance Report

Merging #7491 will not alter performance

Comparing p/clean-up (3a1ef6a) with main (8d89fdc)

Summary

✅ 30 untouched benchmarks

@Boshen
Copy link
Member

Boshen commented Nov 26, 2024

how deep to use macro

Although I ban macros in the rest of the codebase, I allow in this crate to make code easier to read.

So the general guideline is:

  • make the formatter functions easier to read by making everything shorter and more concise.
  • save some key strokes

@Boshen
Copy link
Member

Boshen commented Nov 26, 2024

free to make any changes to the oxc_prettier crate.
#5068 (comment)

@Boshen Can I understand this to mean that I can merge PRs without review?

Or use graphite to stack the PRs, whatever makes you productive. I can review after merge.

@Boshen Boshen added the 0-merge Merge with Graphite Merge Queue label Nov 26, 2024
Copy link
Member

Boshen commented Nov 26, 2024

Merge activity

  • Nov 26, 4: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.
  • Nov 26, 4:27 AM EST: A user added this pull request to the Graphite merge queue.
  • Nov 26, 4:38 AM EST: A user merged this pull request with the Graphite merge queue.

Boshen pushed a commit that referenced this pull request Nov 26, 2024
- Remove `format!` macro
- Rename string related macros, `&'static`: `text!` and `'a`: `dynamic_text!`
- Apply `wrap!` macro instead of manually using `enter|leave_node`
- Introduce `/ir` directory and move `Doc`, `impl Display`, `DocBuilder`, etc.

I'm not yet determined how to, how deep to use macro, but this is first stepping stone... 🥌
- Remove `format!` macro
- Rename string related macros, `&'static`: `text!` and `'a`: `dynamic_text!`
- Apply `wrap!` macro instead of manually using `enter|leave_node`
- Introduce `/ir` directory and move `Doc`, `impl Display`, `DocBuilder`, etc.

I'm not yet determined how to, how deep to use macro, but this is first stepping stone... 🥌
@graphite-app graphite-app bot merged commit 3a1ef6a into main Nov 26, 2024
26 checks passed
@graphite-app graphite-app bot deleted the p/clean-up branch November 26, 2024 09:38
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-prettier Area - Prettier 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