-
Notifications
You must be signed in to change notification settings - Fork 1
SL-18330: In XML formatter, avoid adding call stack depth. #13
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
Conversation
Making LLSDXMLFormatter._elt() accept a lambda was a tricky way to minimize source changes to existing _elt() calls in _ARRAY() and _MAP(). The trouble is that that added function entries for each level of a deeply-nested LLSD structure -- and with our users, we unfortunately do encounter deeply-nested large inventories. Log observed RecursionError failures in AIS. Explicitly write out the individual sequence of calls in _ARRAY(), _MAP() and the top-level _write(), the only callers to pass lambdas to _elt().
CLA Assistant Lite bot: I have read the CLA Document and I hereby sign the CLA 1 out of 2 committers have signed the CLA. |
Codecov Report
@@ Coverage Diff @@
## main #13 +/- ##
==========================================
+ Coverage 90.28% 90.33% +0.04%
==========================================
Files 6 6
Lines 844 848 +4
==========================================
+ Hits 762 766 +4
Misses 82 82
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changes seem fine. Since Log is an employee I think we can look past the CLA check for now.
@bennettgoble I'm not yet clear on what should trigger a new release. I guess I'll create a minor one and hope it helps. |
I have read the CLA Document and I hereby sign the CLA |
Making
LLSDXMLFormatter._elt()
accept a lambda was a tricky way to minimizesource changes to existing
_elt()
calls in_ARRAY()
and_MAP()
. The trouble isthat that added function entries for each level of a deeply-nested LLSD
structure -- and with our users, we unfortunately do encounter deeply-nested
large inventories. Log observed
RecursionError
failures in AIS.Explicitly write out the individual sequence of calls in
_ARRAY()
,_MAP()
andthe top-level
_write()
, the only callers to pass lambdas to_elt()
.