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

DDS-2033: Store troubleshooting doc as markdown, delete PDF #1546

Merged
merged 6 commits into from
Aug 20, 2024

Conversation

aishling-scilifelab
Copy link
Collaborator

@aishling-scilifelab aishling-scilifelab commented Jul 30, 2024

  • Update GitHub Action to do the same for this as is done for the Technical Overview. Built PDF is called dds-troubleshooting.pdf.
  • Update troubleshooting.html to have the correct path and no option to view on GitHub.
  • Delete Troubleshooting.pdf
  • Add troubleshooting.md
  • Update link to troubleshooting doc in README
  • Update Makefile

2. Jira task / GitHub issue

Link to the github issue or add the Jira task ID here.

3. Type of change

What type of change(s) does the PR contain?

Check the relevant boxes below. For an explanation of the different sections, enter edit mode of this PR description template.

  • New feature
    • Breaking: Why / How? Add info here.
    • Non-breaking
  • Database change: Remember the to include a new migration version, or explain here why it's not needed.
  • Bug fix
  • Security Alert fix
    • Package update
      • Major version update
  • Documentation
  • Workflow
  • Tests only

4. Additional information

5. Actions / Scans

Check the boxes when the specified checks have passed.

For information on what the different checks do and how to fix it if they're failing, enter edit mode of this description or go to the PR template.

  • Black
  • Prettier
  • Yamllint
  • Tests
  • CodeQL
  • Trivy
  • Snyk

@aishling-scilifelab aishling-scilifelab force-pushed the DDS-2033-troubleshooting-doc-to-pdf branch from e75bc7e to dbbd1b6 Compare July 30, 2024 12:29
@aishling-scilifelab aishling-scilifelab marked this pull request as ready for review July 30, 2024 12:29
@aishling-scilifelab aishling-scilifelab requested a review from a team July 30, 2024 12:29
@aishling-scilifelab aishling-scilifelab force-pushed the DDS-2033-troubleshooting-doc-to-pdf branch from dbbd1b6 to 04a1c0f Compare July 30, 2024 12:31
Copy link

codecov bot commented Jul 30, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 92.46%. Comparing base (05527d8) to head (992fc91).
Report is 7 commits behind head on dev.

Additional details and impacted files
@@           Coverage Diff           @@
##              dev    #1546   +/-   ##
=======================================
  Coverage   92.46%   92.46%           
=======================================
  Files          29       29           
  Lines        4830     4830           
=======================================
  Hits         4466     4466           
  Misses        364      364           

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

@aishling-scilifelab
Copy link
Collaborator Author

Something that is a note and I possibly should do in a separate branch is that when you are developing locally, you now need to run the make command to build the PDFs. Possibly should do that in the docker compose instead.

Copy link
Member

@rv0lt rv0lt left a comment

Choose a reason for hiding this comment

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

For the workflows file:

  • Name of the job is still build_tech_overview even thought it now handles the troubleshooting as well.

  • Couldn't it, maybe, be a bit cleaner having it in two separate jobs? i.e keep the build_tech_overview and have a new one build_trouble ?

In adition to this last sugestion, it is also probably better to handle them as two separate artifacts. I don't find the documentation very clear to this use case of using multiple paths:
https://github.com/actions/upload-artifact?tab=readme-ov-file#upload-using-multiple-paths-and-exclusions

If multiple paths are provided as input, the least common ancestor of all the search paths will be used as the root directory of the artifact. Exclude paths do not affect the directory structure.

Copy link
Member

@valyo valyo left a comment

Choose a reason for hiding this comment

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

I needed to install the full MacTeX package plus librsvg (maybe we can add it to the brew install command in line 8 of Makefile) in order to successfully run the make command
Apart from that, I agree with Alvaro's comment about having separate jobs

@aishling-scilifelab
Copy link
Collaborator Author

In adition to this last sugestion, it is also probably better to handle them as two separate artifacts. I don't find the documentation very clear to this use case of using multiple paths: https://github.com/actions/upload-artifact?tab=readme-ov-file#upload-using-multiple-paths-and-exclusions

If multiple paths are provided as input, the least common ancestor of all the search paths will be used as the root directory of the artifact. Exclude paths do not affect the directory structure.

Sounds good, I was on the fence about separating but will do.

@aishling-scilifelab aishling-scilifelab force-pushed the DDS-2033-troubleshooting-doc-to-pdf branch 3 times, most recently from 335b6a9 to 59a044b Compare August 13, 2024 12:33
@aishling-scilifelab
Copy link
Collaborator Author

@valyo - will tidy up the commits here before merging but do the actual post-code review changes look good to you?

Makefile Outdated Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
@aishling-scilifelab aishling-scilifelab force-pushed the DDS-2033-troubleshooting-doc-to-pdf branch from 5da1e85 to e46aec4 Compare August 15, 2024 11:16
- Update GitHub Action to do the same for this as is done for the
Technical Overview. Built PDF is called dds-troubleshooting.pdf.
- Update troubleshooting.html to have the correct path and no option to
view on GitHub.
- Delete Troubleshooting.pdf
- Add troubleshooting.md
- Update link to troubleshooting doc in README
- Update Makefile
- Specify that the links to downloadable PDFs is for the CLI.
- Replace reference to redbox (which doesn't exist in the current PDF,
even before the port to markdown)
- Remove mentions of the sysadmins as this is inaccurate. They appear to
be referencing DC sysadmins. Also, in the same section remove
references to Nextcloud.
- These were not added by DDS-2033 but were missing
from the when this Makefile was first introduced.
- Added both basictex and librsvg
Makefile Show resolved Hide resolved
@aishling-scilifelab
Copy link
Collaborator Author

@rv0lt - did your requested changes - can you approve?

@aishling-scilifelab aishling-scilifelab merged commit 5e49f1f into dev Aug 20, 2024
16 checks passed
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