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

Report output #9

Merged
merged 5 commits into from
Dec 29, 2022
Merged

Report output #9

merged 5 commits into from
Dec 29, 2022

Conversation

theztefan
Copy link
Owner

  • Fix: report in JSON format to be uploaded as artifact.
  • Options to configure the report format: JSON, PDF

Copy link
Collaborator

@gateixeira gateixeira left a comment

Choose a reason for hiding this comment

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

Just have two comments/questions

These changes will conflict a lot with the changes I am making cause I am changing a little bit the way we are handling types. But leave it to me, I can merge it together after this gets merged.

@@ -8,16 +8,20 @@ inputs:
default: srdemo-demo
org:
description: "The organisation to generate the report for"
required: false
required: true
Copy link
Collaborator

Choose a reason for hiding this comment

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

is there a reason why you changed these fields to required: true but still kept the default value?

Copy link
Owner Author

Choose a reason for hiding this comment

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

Yep, I thought of you when I changed this. 😄

@@ -127,6 +129,197 @@ export function prepareSummary(report: Report): void {
]);
}

export function preparePdf(report: Report): jsPDF {
Copy link
Collaborator

Choose a reason for hiding this comment

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

have you considered making it a bit more generic to generate the reports? It is repeating a lot of data between JSON and PDF

Copy link
Owner Author

Choose a reason for hiding this comment

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

Well one function was preparing and writing markdown using core.summary and the other one is preprating a jsPDF() object. The object is then passed to be written to a file.

That is the only difference. The data is very much the same. There should be a more generic way to do this but didn't get to it in this iteration.

@theztefan theztefan merged commit 5299b3e into main Dec 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants