-
Notifications
You must be signed in to change notification settings - Fork 38
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
docs: Add Deprecated names of variables #574
base: dev
Are you sure you want to change the base?
Conversation
remove FP_DEF_TEMPLATE for OpenROADStep config_vars Signed-off-by: Kareem Farid <[email protected]>
Metric comparisons are in beta. Please report bugs under the issues tab.
Full tables ► https://gist.github.com/openlane-bot/50c91a8a5570f600f996a53bfa539557 |
Iterate on set of unique Variables when generating docs for a step Update __hash__ for Variable to avoid unhashable list Signed-off-by: Kareem Farid <[email protected]>
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.
To implement this properly, you have to create HTML tables in place of the Markdown tables. There's almost no way around it.
Additionally while you're doing that, I think the best approach would be to have a collapsible below the current name- another column occupies precious horizontal real estate
%for var in pdk_variables: | ||
| `${var.name}`{#${var._get_docs_identifier()}} | ${var.type_repr_md(for_document=True)} | ${var.desc_repr_md()} | ${var.units or ""} | | ||
| `${var.name}`{#${var._get_docs_identifier()}} | ${var.type_repr_md(for_document=True)} | ${var.desc_repr_md()} | ${var.units or ""} | ${var.get_deprecated_names_md()|join("<br>")} | |
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.
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.
Where is that? The table renders fine in readthedocs.
%for var in scl_variables: | ||
| `${var.name}`{#${var._get_docs_identifier()}} | ${var.type_repr_md(for_document=True)} | ${var.desc_repr_md()} | ${var.units or ""} | | ||
| `${var.name}`{#${var._get_docs_identifier()}} | ${var.type_repr_md(for_document=True)} | ${var.desc_repr_md()} | ${var.units or ""} | ${var.get_deprecated_names_md()|join("<br>")} | |
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.
ditto
%for var in option_variables: | ||
| [`${var.name}`]{#${var._get_docs_identifier()}} | ${var.type_repr_md(for_document=True)} | ${var.desc_repr_md()} | `${var.default}` | ${var.units or ""} | | ||
| [`${var.name}`]{#${var._get_docs_identifier()}} | ${var.type_repr_md(for_document=True)} | ${var.desc_repr_md()} | `${var.default}` | ${var.units or ""} | ${var.get_deprecated_names_md()|join("<br>")} | |
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.
ditto
""" | ||
) | ||
for var in flow_config_vars: | ||
units = var.units or "" | ||
pdk_superscript = "<sup>PDK</sup>" if var.pdk else "" | ||
result += f"| `{var.name}`{{#{var._get_docs_identifier(Self.__name__)}}}{pdk_superscript} | {var.type_repr_md()} | {var.desc_repr_md()} | `{var.default}` | {units} |\n" | ||
result += f"| `{var.name}`{{#{var._get_docs_identifier(Self.__name__)}}}{pdk_superscript} | {var.type_repr_md()} | {var.desc_repr_md()} | `{var.default}` | {units} | {'<br>'.join(var.get_deprecated_names_md())} |\n" |
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.
ditto
units = var.units or "" | ||
pdk_superscript = "<sup>PDK</sup>" if var.pdk else "" | ||
result += f"| `{var.name}`{{#{var._get_docs_identifier(Self.id)}}}{pdk_superscript} | {var.type_repr_md(for_document=True)} | {var.desc_repr_md()} | `{var.default}` | {units} |\n" | ||
result += f"| `{var.name}`{{#{var._get_docs_identifier(Self.id)}}}{pdk_superscript} | {var.type_repr_md(for_document=True)} | {var.desc_repr_md()} | `{var.default}` | {units} | {'<br>'.join(var.get_deprecated_names_md())} |\n" |
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.
ditto
There was an issue in forming the markdown table which is now fixed. As for the collapsable, I don't think it is necessary. Deprecated names are added so that they are viewable when search the docs - or using Ctrl+f. The PR accomplishes that. |
Ctrl + F works with collapsible though, no? |
Apparently not. The importing part is not matched by Ctrl + F in the current docs Edit: |
Variables
Variable
Docs
Variable
Variable
for a specificStep
resolves #526