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

Do not duplicate log messages #19984

Merged
merged 2 commits into from
Oct 2, 2023
Merged

Do not duplicate log messages #19984

merged 2 commits into from
Oct 2, 2023

Conversation

lubosdz
Copy link
Contributor

@lubosdz lubosdz commented Sep 28, 2023

Q A
Is bugfix? ✔️
New feature?
Breaks BC?

There was missing EOL introduced in #16208, then fixed in #19940, but still there is redundant variable $trimmedText which duplicates in memory log messages possibly exhausting memory on large logs. This PR is only cleanup to remove it.

@what-the-diff
Copy link

what-the-diff bot commented Sep 28, 2023

PR Summary

  • Modification to the 'export' function in FileTarget.php
    • The logic for verifying if there are no messages to export has been improved. Previously, we relied on detecting if the 'trimmedText' variable was empty. The new change, however, checks if the 'text' variable itself - once spaces are removed - is empty. This small but significant enhancement ensures that our export function operates more efficiently and accurately, considering scenarios where 'text' might not be empty but contains only white space.

@codecov
Copy link

codecov bot commented Sep 28, 2023

Codecov Report

All modified lines are covered by tests ✅

Comparison is base (98c4bb8) 48.48% compared to head (7f3397a) 48.94%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master   #19984      +/-   ##
==========================================
+ Coverage   48.48%   48.94%   +0.45%     
==========================================
  Files         445      445              
  Lines       42336    42817     +481     
==========================================
+ Hits        20527    20956     +429     
- Misses      21809    21861      +52     
Files Coverage Δ
framework/log/FileTarget.php 62.65% <100.00%> (-0.45%) ⬇️

... and 15 files with indirect coverage changes

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

@terabytesoftw terabytesoftw added the status:code review The pull request needs review. label Sep 28, 2023
@terabytesoftw
Copy link
Member

terabytesoftw commented Sep 28, 2023

Add line in changelog,

Thks,

@terabytesoftw terabytesoftw added this to the 2.0.50 milestone Sep 28, 2023
@lubosdz
Copy link
Contributor Author

lubosdz commented Sep 29, 2023

Changelog added, thanx.

@bizley bizley added type:enhancement and removed status:code review The pull request needs review. labels Oct 2, 2023
@bizley bizley merged commit 75f37a7 into yiisoft:master Oct 2, 2023
49 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants