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

Simplify Parser and Introduce Formatter #238

Merged
merged 12 commits into from
Jul 30, 2024
Merged

Simplify Parser and Introduce Formatter #238

merged 12 commits into from
Jul 30, 2024

Conversation

cpisciotta
Copy link
Owner

@cpisciotta cpisciotta commented Jan 14, 2024

Changes

Major

Simplify Parser to exclusively implement its intended functionality: parsing. Separate formatting-related logic into a new type, Formatter.

Move the beautify method from OutputRendering to Formatter, delete the unnecessary extension on String, and update JunitReporter as necessary. Update XCBeautifier and the executable to wrap Parser, Formatter, and output-related logic.

Minor

  • Make types package-accessible, as necessary.
  • Update unit tests to support new implementation.

Testing

Performance

Old (Seconds) New (Seconds) Difference (%)
Run 1 4.84 4.88 -
Run 2 4.87 4.73 -
Run 3 4.82 4.79 -
Run 4 4.93 4.73 -
Run 5 4.87 4.75 -
Average 4.866 4.776 -1.87%

Copy link

codecov bot commented Feb 25, 2024

Codecov Report

Attention: Patch coverage is 86.40351% with 31 lines in your changes missing coverage. Please review.

Project coverage is 86.81%. Comparing base (8c8be6a) to head (335276c).

Files Patch % Lines
Sources/XcbeautifyLib/Formatter.swift 90.20% 19 Missing ⚠️
Sources/XcbeautifyLib/XCBeautifier.swift 0.00% 9 Missing ⚠️
Sources/XcbeautifyLib/Parser.swift 75.00% 2 Missing ⚠️
Sources/XcbeautifyLib/CaptureGroups.swift 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #238      +/-   ##
==========================================
- Coverage   87.79%   86.81%   -0.99%     
==========================================
  Files          14       14              
  Lines        1614     1600      -14     
==========================================
- Hits         1417     1389      -28     
- Misses        197      211      +14     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@cpisciotta cpisciotta force-pushed the DeleteTestSummary branch 2 times, most recently from 5e1180a to c342784 Compare March 1, 2024 04:48
@cpisciotta cpisciotta force-pushed the DeleteTestSummary branch 6 times, most recently from 492afd1 to 84c210e Compare March 15, 2024 02:31
@cpisciotta cpisciotta force-pushed the DeleteTestSummary branch 2 times, most recently from cda9383 to 6822557 Compare April 27, 2024 02:28
@cpisciotta cpisciotta force-pushed the ParserRefactor branch 4 times, most recently from d4434f8 to 837b9b7 Compare April 27, 2024 03:38
@cpisciotta cpisciotta force-pushed the ParserRefactor branch 12 times, most recently from 3a7cad1 to d1bad2e Compare July 30, 2024 20:46
@cpisciotta cpisciotta changed the title [WIP] Refactor Parser Refactor Parser Jul 30, 2024
@cpisciotta cpisciotta marked this pull request as ready for review July 30, 2024 20:52
@cpisciotta cpisciotta changed the title Refactor Parser Simplify Parser and Introduce Formatter Jul 30, 2024
@cpisciotta cpisciotta merged commit 80d0acb into main Jul 30, 2024
9 of 11 checks passed
@cpisciotta cpisciotta deleted the ParserRefactor branch July 30, 2024 23:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant