-
Notifications
You must be signed in to change notification settings - Fork 81
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
Split sphinxHtmlToMarkdown into helper functions #596
Conversation
imageDestination?: string; | ||
imageDestination: string; | ||
// url links to a fixed version and ending in / | ||
// https://github.com/Qiskit/qiskit-ibm-runtime/tree/0.9.2/ | ||
baseSourceUrl?: string; | ||
releaseNotesTitle?: string; | ||
baseSourceUrl: string; | ||
releaseNotesTitle: string; |
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.
These were only optional for the sake of testing. But that's a dangerous strategy because it's really important that production never forget to set the arguments.
addLanguageClassToCodeBlocks($, $main); | ||
replaceSourceLinksWithGitHub($, $main, baseSourceUrl); | ||
convertRubricsToHeaders($, $main); | ||
processSimpleFieldLists($, $main); |
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.
I don't like this name very much, but I still don't understand well all the edge cases of what this function is doing. We should rename it once we have a better understanding of the semantic significance.
convertRubricsToHeaders($, $main); | ||
processSimpleFieldLists($, $main); | ||
removeColons($main); | ||
preserveMathBlockWhitespace($, $main); |
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.
This used to happen later in the sequence. I moved it here because it "fits" better here with the other processing.
} | ||
|
||
/** | ||
* Convert sphinx-design tabs. |
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.
This comment is new.
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.
I find this much easier to read, thank you!
Part of Qiskit#223. There are still improvements we can make like better testing. This PR tries to avoid making code changes and only moves the code to helper functions.
Part of #223. There are still improvements we can make like better testing.
This PR tries to avoid making code changes and only moves the code to helper functions.