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(span): combine Span type and impls in 1 file #8900

Merged

Conversation

overlookmotel
Copy link
Contributor

@overlookmotel overlookmotel commented Feb 4, 2025

Pure refactor. Combine all code related to Span into a single file.

The motivation of splitting it into 2 files originally was that oxc_ast_tools could not handle impls in same file as type definitions. That restriction is now lifted, so there's no need for the artificial split now.

@github-actions github-actions bot added A-ast-tools Area - AST tools C-cleanup Category - technical debt or refactoring. Solution not expected to change behavior labels Feb 4, 2025
Copy link
Contributor Author

overlookmotel commented Feb 4, 2025


How to use the Graphite Merge Queue

Add either label to this PR to merge it via the merge queue:

  • 0-merge - adds this PR to the back of the merge queue
  • hotfix - for urgent hot fixes, skip the queue and merge this PR next

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

An organization admin has enabled the Graphite Merge Queue in this repository.

Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue.

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

@overlookmotel overlookmotel marked this pull request as ready for review February 4, 2025 15:14
@overlookmotel overlookmotel force-pushed the 02-04-refactor_span_combine_span_type_and_impls_in_1_file branch from 86d83e9 to 4de0f57 Compare February 4, 2025 15:16
@overlookmotel overlookmotel changed the base branch from 02-04-fix_ast_fix_lifetimes_on_custom_serialize_impls to 02-04-refactor_ast_estree_via_on_struct_fields_implement_from_ February 4, 2025 15:16
This was referenced Feb 4, 2025
Copy link

codspeed-hq bot commented Feb 4, 2025

CodSpeed Performance Report

Merging #8900 will not alter performance

Comparing 02-04-refactor_span_combine_span_type_and_impls_in_1_file (e930cae) with main (5cb8466)

Summary

✅ 33 untouched benchmarks

@Boshen Boshen added the 0-merge Merge with Graphite Merge Queue label Feb 4, 2025
Copy link
Member

Boshen commented Feb 4, 2025

Merge activity

  • Feb 4, 10:52 AM EST: The merge label '0-merge' was detected. This PR will be added to the Graphite merge queue once it meets the requirements.
  • Feb 4, 5:21 PM EST: The Graphite merge queue removed this pull request due to downstack failures on PR #8881.
  • Feb 4, 5:22 PM EST: The merge label '0-merge' was detected. This PR will be added to the Graphite merge queue once it meets the requirements.
  • Feb 4, 10:22 PM UTC: The merge label '0-merge' was removed. This PR will no longer be merged by the Graphite merge queue
  • Feb 4, 8:57 PM EST: The merge label '0-merge' was detected. This PR will be added to the Graphite merge queue once it meets the requirements.
  • Feb 4, 8:57 PM EST: A user added this pull request to the Graphite merge queue.
  • Feb 4, 9:03 PM EST: A user merged this pull request with the Graphite merge queue.

@overlookmotel overlookmotel force-pushed the 02-04-refactor_ast_estree_via_on_struct_fields_implement_from_ branch from 80264d4 to 74d5176 Compare February 4, 2025 21:56
@overlookmotel overlookmotel force-pushed the 02-04-refactor_span_combine_span_type_and_impls_in_1_file branch from 4de0f57 to c208441 Compare February 4, 2025 21:56
@graphite-app graphite-app bot removed the 0-merge Merge with Graphite Merge Queue label Feb 4, 2025
@graphite-app graphite-app bot force-pushed the 02-04-refactor_ast_estree_via_on_struct_fields_implement_from_ branch from 74d5176 to f22d18b Compare February 4, 2025 23:13
@graphite-app graphite-app bot force-pushed the 02-04-refactor_span_combine_span_type_and_impls_in_1_file branch from c208441 to c266c49 Compare February 4, 2025 23:14
@overlookmotel overlookmotel force-pushed the 02-04-refactor_span_combine_span_type_and_impls_in_1_file branch from c266c49 to 6eb24da Compare February 5, 2025 01:38
@overlookmotel overlookmotel changed the base branch from 02-04-refactor_ast_estree_via_on_struct_fields_implement_from_ to graphite-base/8900 February 5, 2025 01:45
@overlookmotel overlookmotel force-pushed the 02-04-refactor_span_combine_span_type_and_impls_in_1_file branch from 6eb24da to 2b511fa Compare February 5, 2025 01:50
@overlookmotel overlookmotel changed the base branch from graphite-base/8900 to main February 5, 2025 01:51
@overlookmotel overlookmotel force-pushed the 02-04-refactor_span_combine_span_type_and_impls_in_1_file branch from 2b511fa to 589d226 Compare February 5, 2025 01:51
@overlookmotel overlookmotel added the 0-merge Merge with Graphite Merge Queue label Feb 5, 2025
Pure refactor. Combine all code related to `Span` into a single file.

The motivation of splitting it into 2 files originally was that `oxc_ast_tools` could not handle `impl`s in same file as type definitions. That restriction is now lifted, so there's no need for the artificial split now.
@graphite-app graphite-app bot force-pushed the 02-04-refactor_span_combine_span_type_and_impls_in_1_file branch from 589d226 to e930cae Compare February 5, 2025 01:57
@graphite-app graphite-app bot merged commit e930cae into main Feb 5, 2025
25 checks passed
@graphite-app graphite-app bot deleted the 02-04-refactor_span_combine_span_type_and_impls_in_1_file branch February 5, 2025 02:03
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-tools Area - AST tools 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