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

make the LogCollector fields public #1075

Merged
merged 1 commit into from
Apr 27, 2024

Conversation

kevinheavey
Copy link

Problem

I need to use Refcell::replace on the collected logs instead of .take(), because I want to control the bytes_limit. This requires being able to construct a plain LogCollector instead of using new_ref_with_limit() which returns an Rc<RefCell<LogCollector>>.

I'd also like to be able to use a preallocated Vec for the messages field. At this point it seems simplest to just make all four fields public.

Summary of Changes

Makes the LogCollector fields pub

@mergify mergify bot requested a review from a team April 26, 2024 15:55
Copy link

@buffalojoec buffalojoec left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wfm!

@buffalojoec buffalojoec added CI Pull Request is ready to enter CI automerge automerge Merge this Pull Request automatically once CI passes labels Apr 27, 2024
@anza-team anza-team removed CI Pull Request is ready to enter CI labels Apr 27, 2024
@codecov-commenter
Copy link

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 82.1%. Comparing base (d9d52be) to head (2db28fb).
Report is 11 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff            @@
##           master    #1075     +/-   ##
=========================================
- Coverage    82.1%    82.1%   -0.1%     
=========================================
  Files         868      868             
  Lines      234074   234126     +52     
=========================================
+ Hits       192373   192408     +35     
- Misses      41701    41718     +17     

@mergify mergify bot merged commit 075c790 into anza-xyz:master Apr 27, 2024
44 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automerge automerge Merge this Pull Request automatically once CI passes community need:merge-assist
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants