Skip to content
This repository was archived by the owner on May 5, 2025. It is now read-only.

Remove more unused Report code, change serde code to use str#575

Merged
Swatinem merged 1 commit intomainfrom
swatinem/report-clean-serde
Mar 18, 2025
Merged

Remove more unused Report code, change serde code to use str#575
Swatinem merged 1 commit intomainfrom
swatinem/report-clean-serde

Conversation

@Swatinem
Copy link
Copy Markdown
Contributor

I removed the users of the to_archive/to_database functions, so lets remove those, along with other functions.

Another change is to change the serde code to use str. As strange as it might sound, but I believe that python is optimizing strings a bit better, and avoids copying all the string contents around in some cases.

This also cleans up the tests a bit.

@Swatinem Swatinem self-assigned this Mar 18, 2025
@seer-by-sentry
Copy link
Copy Markdown

seer-by-sentry Bot commented Mar 18, 2025

✅ Sentry found no issues in your recent changes ✅

@codecov
Copy link
Copy Markdown

codecov Bot commented Mar 18, 2025

Codecov Report

Attention: Patch coverage is 95.23810% with 1 line in your changes missing coverage. Please review.

Project coverage is 88.88%. Comparing base (d84177b) to head (a51554c).
Report is 1 commits behind head on main.

✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
shared/reports/resources.py 88.88% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #575      +/-   ##
==========================================
- Coverage   88.88%   88.88%   -0.01%     
==========================================
  Files         457      457              
  Lines       12867    12840      -27     
  Branches     1469     1467       -2     
==========================================
- Hits        11437    11413      -24     
+ Misses       1126     1123       -3     
  Partials      304      304              
Flag Coverage Δ
shared-docker-uploader 88.88% <95.23%> (+0.13%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

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

@codecov-notifications
Copy link
Copy Markdown

codecov-notifications Bot commented Mar 18, 2025

Codecov Report

Attention: Patch coverage is 95.23810% with 1 line in your changes missing coverage. Please review.

✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
shared/reports/resources.py 88.88% 1 Missing ⚠️

📢 Thoughts on this report? Let us know!

@codspeed-hq
Copy link
Copy Markdown

codspeed-hq Bot commented Mar 18, 2025

CodSpeed Performance Report

Merging #575 will improve performances by 24.67%

Comparing swatinem/report-clean-serde (a51554c) with main (d84177b)

Summary

⚡ 2 improvements
✅ 7 untouched benchmarks

Benchmarks breakdown

Benchmark BASE HEAD Change
test_parse_shallow 12.4 ms 9.9 ms +24.67%
test_report_serialize 7.1 ms 5.9 ms +19.76%

@Swatinem Swatinem requested a review from a team March 18, 2025 12:12
@Swatinem Swatinem marked this pull request as ready for review March 18, 2025 12:12
@Swatinem
Copy link
Copy Markdown
Contributor Author

This fixes most of the regressions from #553 (comment) it would seem.

@Swatinem Swatinem added this pull request to the merge queue Mar 18, 2025
Merged via the queue into main with commit 9d1b9b1 Mar 18, 2025
13 checks passed
@Swatinem Swatinem deleted the swatinem/report-clean-serde branch March 18, 2025 13:05
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants