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

Fix GHAs for DOCX and PDF downloads #717

Merged
merged 11 commits into from
Dec 14, 2023
Merged

Conversation

Jeff-Thompson12
Copy link
Collaborator

Addresses #702

Copy link

codecov bot commented Dec 5, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (93f80f1) 77.17% compared to head (e4b004e) 78.14%.

❗ Current head e4b004e differs from pull request most recent head 5a78a0f. Consider uploading reports for the commit 5a78a0f to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##              dev     #717      +/-   ##
==========================================
+ Coverage   77.17%   78.14%   +0.96%     
==========================================
  Files          33       33              
  Lines        4872     4872              
==========================================
+ Hits         3760     3807      +47     
+ Misses       1112     1065      -47     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@Jeff-Thompson12
Copy link
Collaborator Author

I have investigated printing the HTML reports to PDF. While some improvements can be made to make the reports nicer to print interactively, I do not think the work to get the reports to work programmatically is worth the effort (at least not in the short term).

Efforts would be better suited to determining which LaTeX packages are necessary to be able to print the DOCX and PDF versions of the report and to provide some user feedback if those packages are missing from the environment where the application is running.

@Jeff-Thompson12 Jeff-Thompson12 added the help wanted Extra attention is needed label Dec 7, 2023
@Jeff-Thompson12
Copy link
Collaborator Author

I was able to get the PDF reports to work on the GHAs by installing the texlive-xetex package to get XeLatex.

@Jeff-Thompson12
Copy link
Collaborator Author

I think this gives us a path forward to getting the downloads to work in deployments. Hopefully the steps to get the downloads to work on the GHAs will be similar to what will be needed on the deployment servers. The downside is that users will have to get their respective admins to install the necessary dependencies.

@Jeff-Thompson12 Jeff-Thompson12 removed the help wanted Extra attention is needed label Dec 13, 2023
@AARON-CLARK
Copy link
Collaborator

To summarize a discussion with @Jeff-Thompson12 offline:

It's still on draft because I haven't actually fully resolved the deployment downloads.

I still have not created guidance for setting up that environment so that the reports can download...

... or figuring out how to determine if a report can compile. I feel like at least we should disable downloads that can't compile (if we can). Aka, provide some user feedback if those packages are missing from the environment where the application is running.

@Jeff-Thompson12 Jeff-Thompson12 changed the title Fix DOCX and PDF downloads Fix GHAs for DOCX and PDF downloads Dec 14, 2023
@Jeff-Thompson12
Copy link
Collaborator Author

Per discussion today, I am changing the scope of this PR since the changes made benefit our workflow. I will open a new PR to tackle the download issues when the app is deployed.

@Jeff-Thompson12 Jeff-Thompson12 marked this pull request as ready for review December 14, 2023 20:30
Copy link
Contributor

@Robert-Krajcik Robert-Krajcik left a comment

Choose a reason for hiding this comment

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

LGTM

@Robert-Krajcik Robert-Krajcik merged commit a289949 into dev Dec 14, 2023
3 checks passed
@AARON-CLARK AARON-CLARK deleted the jt-702-fix_deploy_dwnlds branch January 17, 2024 14:30
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.

3 participants