-
Notifications
You must be signed in to change notification settings - Fork 4.5k
Bugfix within shorthand example generation for map-type parameters in documentation #9982
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
base: v2
Are you sure you want to change the base?
Conversation
|
For reviewer, some of the 38 added shorthand syntax examples are relatively long/complex due to the increase of the max stack-size from 3 to 4. I want to get feedback on this. For example, below is a shorthand syntax example that will be added to securityhub/create-insight if we merge this as-is. It contains 10,158 characters. For reference, that largest shorthand syntax example currently published is 4,982 characters long, located at https://docs.aws.amazon.com/cli/latest/reference/timestream-influxdb/create-db-parameter-group.html. My proposal is to implement a second threshold, that prevents shorthand syntax from being generated if it exceeds 5,000 characters. I'm open to adding this to this PR if the code reviewer agrees with this proposal.
Text: According to code comments, there was a long-term plan to create an improved way of displaying complex shorthand syntax. |
|
Can we compare diffs between current generated HTML and new to make sure only expected changes are being made? |
hssyoo
left a comment
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.
Bugfix looks good, not sure about changing max stack
|
|
||
| _DONT_DOC = object() | ||
| _MAX_STACK = 3 | ||
| _MAX_STACK = 4 |
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.
It seems like setting _MAX_STACK to 3 was an intentional choice: #1467
Instead of defining more logic to remove long shorthand syntax if it's over a certain threshold, I'm inclined to just not change _MAX_STACK unless we think there's enough value coming from net-new shorthand syntax docs.
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.
Instead of defining more logic to remove long shorthand syntax if it's over a certain threshold, I'm inclined to just not change _MAX_STACK unless we think there's enough value coming from net-new shorthand syntax docs.
Fair. Just to clarify my original perspective, by making the bugfix so that map adds to the stack, some shorthand syntax that had map as the parameter type will now have a stack size of 4 instead of 3, now causing those to no longer show in docs. Technically these examples always had a stack size of 4 in theory (map + 3 other levels), just that map never added to the stack, so it was actually 3 in practice.
So maybe the 61 shorthand examples that will be removed technically should not have been generated previously. I'll revert to 3 in next revision. Just leaving that context here.

Related Issues:
Description of changes:
map, and the value is a structure or list, include curly-braces or square-brackets around the value. This change is consistent with how structures are handled.3to4. This threshold controls which parameters receive a generated shorthand example. Without this increase, 61 shorthand examples would have been removed from the docs. As a consequence of this increase, 38 shorthand examples will be added to the docs (see Description of Impact below).Description of tests:
Notes:
This PR supersedes #7286, which did not increase the max stack size to 4, and would have caused 61 shorthand examples for being removed.
Description of impact:
Two files attaches below describe the full set of docs changes that will be caused by this change:
map.shorthand_diff_4stack_unique_summary.txt
shorthand_diff_4stack_summary.txt