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

[alert_handler] Convert theory of operation to template #19157

Merged
merged 1 commit into from
Jul 11, 2023

Conversation

a-will
Copy link
Contributor

@a-will a-will commented Jul 10, 2023

The theory of operation document referenced a particular instance's generated core's parameters in a table. Change the table header to not explicitly refer to Earl Grey and populate some of the values from template parameters.

Note: The conversion to Mako template caused lines beginning with # to become comments, so I have changed the syntax to use the HTML variant.

@a-will a-will requested a review from msfschaffner July 10, 2023 18:32
@a-will a-will added the Type:Cleanup Cleanup tasks label Jul 10, 2023
@a-will a-will marked this pull request as ready for review July 10, 2023 23:45
@a-will a-will requested a review from hcallahan-lowrisc July 10, 2023 23:45
@a-will
Copy link
Contributor Author

a-will commented Jul 10, 2023

This was mostly motivated by a multi-top annoyance, where the new top still had Earl Grey references.

Copy link
Contributor

@hcallahan-lowrisc hcallahan-lowrisc left a comment

Choose a reason for hiding this comment

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

Table changes LGTM.

For the headings, you can use the trick in #3458

Thanks!

@msfschaffner
Copy link
Contributor

We may have to pull in the top-name into IPgen in the future to uniquify generated IPs via a pre- or suffix (e.g. top_earlgrey_alert_handler, and that would allow us to create top-specific documentation in a somewhat more convenient way.

For now however I think this is perfectly fine.

Copy link
Contributor

@msfschaffner msfschaffner left a comment

Choose a reason for hiding this comment

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

LGTM (modulo @hcallahan-lowrisc's suggestion)!

@a-will
Copy link
Contributor Author

a-will commented Jul 11, 2023

Table changes LGTM.

For the headings, you can use the trick in #3458

Thanks!

I don't see how wrapping the Markdown header shorthand in a <%text> Mako tag is any better than the unadulterated HTML / Markdown header tag, haha. But if that's the preference, it's no problem!

The theory of operation document referenced a particular instance's
parameters in a table. Change the table header to not explicitly refer
to Earl Grey and populate some of the values from template parameters.

Signed-off-by: Alexander Williams <[email protected]>
@a-will
Copy link
Contributor Author

a-will commented Jul 11, 2023

Table changes LGTM.
For the headings, you can use the trick in #3458
Thanks!

I don't see how wrapping the Markdown header shorthand in a <%text> Mako tag is any better than the unadulterated HTML / Markdown header tag, haha. But if that's the preference, it's no problem!

Ah, it does minimize the diff for the generated core doc, though. :)

@a-will a-will force-pushed the alert-handler-ipgen branch from 990a0f6 to 90f8a64 Compare July 11, 2023 17:33
@a-will a-will merged commit 2c6d825 into lowRISC:master Jul 11, 2023
@a-will a-will deleted the alert-handler-ipgen branch July 11, 2023 20:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants