-
Notifications
You must be signed in to change notification settings - Fork 18
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
Axio runner updates #213
base: main
Are you sure you want to change the base?
Axio runner updates #213
Conversation
✅ Deploy Preview for authzen-todo canceled.
|
I'm pretty sure the HTML updates will break the markdown. |
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.
Overall I think there's a misunderstanding of what this is meant to do.
The code is meant to generate valid markdown (which can include HTML) in order to generate markdown that can be processed by the docusaurus toolchain to create presentable pages with tables in them.
I believe the original intent was to make some changes like "don't use green for fail", which I think would be a one-line change.
I think this PR should be closed.
@@ -25,6 +25,7 @@ coverage | |||
npm-debug.log* | |||
yarn-debug.log* | |||
yarn-error.log* | |||
yarn.lock |
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.
yarn.lock should always be checked in to ensure that the dependencies are predictably installed
@@ -3,7 +3,7 @@ | |||
.idea | |||
|
|||
# Dependencies | |||
*/node_modules | |||
node_modules |
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.
not sure why this change is needed
@@ -6,12 +6,12 @@ const AUTHZEN_PDP_URL = | |||
const AUTHZEN_PDP_API_KEY = process.env.AUTHZEN_PDP_API_KEY; | |||
|
|||
enum OutputTypes { | |||
MARKDOWN, | |||
HTML, |
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.
the output is actually markdown with embedded HTML. I don't think this change makes sense (at least to me)
<td> | ||
|
||
`; | ||
table += "```js\r\n" + item.request + "\r\n```\r\n\r\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.
this is valid markdown, doesn't make sense to replace it IMO
|
||
// Return table | ||
return table; | ||
} | ||
|
||
function wrapHTML(body) { |
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'm pretty sure this will break the markdown.
As discussed on the call this week, I updated the runner to indicate html rather than markdown. I cleaned up the html output, made it a valid HTML page