Skip to content

Test String Hierarchy Body Model Builder#73

Open
cld99 wants to merge 8 commits into
release-v2.0from
test/string-hierarchy-body-model-builder
Open

Test String Hierarchy Body Model Builder#73
cld99 wants to merge 8 commits into
release-v2.0from
test/string-hierarchy-body-model-builder

Conversation

@cld99
Copy link
Copy Markdown

@cld99 cld99 commented Mar 20, 2025

No description provided.

@cld99 cld99 changed the base branch from main to release-v2.0 March 20, 2025 14:19
@cld99 cld99 requested review from johnshi314 and lhmcgann April 6, 2025 17:30
Copy link
Copy Markdown
Owner

@lhmcgann lhmcgann left a comment

Choose a reason for hiding this comment

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

Great job! Just added a couple small test cases, comments, and cleanup!

if (StringHierarchySpec.TryParseOptionedRegionName(
parentOptionedRegionSpec, out _, out string optionStr) &&
this._availableBaseRegions.TryGetValue(parentOptionedRegionSpec,
out var region))
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Just out of curiosity: why flip these two? what broke?

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

flipped so the parsing happened even if parentOptionedRegionSpec not in available base regions. both orders work, this one just easier for testing

}
catch (System.Reflection.TargetInvocationException e)
{
throw e.InnerException;
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

again just curious: why the inner exception?

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

whenever an exception is thrown in a reflective call, it wraps the exception, so need to unwrap it to see them

@lhmcgann lhmcgann force-pushed the test/string-hierarchy-body-model-builder branch from c25c61c to 69ed547 Compare April 5, 2026 18:44
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.

2 participants