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

feat: add quarto cheat sheet setup to docs build actions #528

Merged
merged 42 commits into from
Aug 26, 2024

Conversation

Revathyvenugopal162
Copy link
Contributor

@Revathyvenugopal162 Revathyvenugopal162 commented Jul 31, 2024

@ansys-reviewer-bot
Copy link
Contributor

Thanks for opening a Pull Request. If you want to perform a review write a comment saying:

@ansys-reviewer-bot review

@github-actions github-actions bot added the enhancement General improvements to existing features label Jul 31, 2024
Copy link
Member

@jorgepiloto jorgepiloto left a comment

Choose a reason for hiding this comment

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

Just some name suggestion:

doc-build/action.yml Outdated Show resolved Hide resolved
@Revathyvenugopal162 Revathyvenugopal162 force-pushed the feat/add-quarto-pyansys-actions branch 3 times, most recently from 78f8f7c to 9141038 Compare August 7, 2024 17:11
Copy link
Member

Choose a reason for hiding this comment

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

This is great! I am more concerned about the versions showing up in the file. Any chance we can use Jinja here? We could have a cheat_sheet.qmd.tpl containing a {{ version }}. We render this via conf.py hooks so then it is built into the final cheatsheet. Also, we could include the code snippets we already have.

I know it is a bit of a burden but it alleviates all the maintenance of the docs.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I understand this might be a bit challenging to implement, but I can give it a try.

Copy link
Member

@jorgepiloto jorgepiloto left a comment

Choose a reason for hiding this comment

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

This is great! Just left a comment on the automation of cheatsheets for this repo.

Copy link
Member

@jorgepiloto jorgepiloto left a comment

Choose a reason for hiding this comment

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

Thanks for the hard work with this, @Revathyvenugopal162. I just left some minor comments.

_doc-build-windows/action.yml Outdated Show resolved Hide resolved
_doc-build-linux/action.yml Outdated Show resolved Hide resolved
doc/source/cheat_sheet.jinja Outdated Show resolved Hide resolved
doc/source/cheat_sheet.jinja Outdated Show resolved Hide resolved
doc/source/cheat_sheet.jinja Outdated Show resolved Hide resolved
doc/source/conf.py Outdated Show resolved Hide resolved
Copy link
Member

Choose a reason for hiding this comment

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

Do we really need to commit this file?

doc/source/cheat_sheet.jinja Outdated Show resolved Hide resolved
doc/source/cheat_sheet.jinja Outdated Show resolved Hide resolved
@jorgepiloto
Copy link
Member

The syntax highlight for the YML code blocks in the cheatsheet seems to be the one for Python. Is it possible to change it to YML?

Copy link
Member

@jorgepiloto jorgepiloto left a comment

Choose a reason for hiding this comment

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

Fixing the highlight.

doc/source/cheat_sheet.jinja Outdated Show resolved Hide resolved
@jorgepiloto jorgepiloto marked this pull request as ready for review August 14, 2024 12:54
@jorgepiloto jorgepiloto requested a review from a team as a code owner August 14, 2024 12:54
@jorgepiloto
Copy link
Member

Thanks for adding all the suggestions, @Revathyvenugopal162. Let's update the references and merge this.

Comment on lines 226 to 230
- name: "Install Jupyter for Quarto"
if: ${{ inputs.needs-quarto == 'true' }}
shell: bash
run: |
python -m pip install jupyter nbformat
Copy link
Member

Choose a reason for hiding this comment

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

Is this required for Quarto to operate? This may be in conflict with packages declared inside of the libraries triggering the actions.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We need jupyter to convert qmd files to pdf format

Copy link
Member

Choose a reason for hiding this comment

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

As discussed, projects should be the ones adding their doc-extensions in the required extra target.

Copy link
Contributor Author

@Revathyvenugopal162 Revathyvenugopal162 Aug 26, 2024

Choose a reason for hiding this comment

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

sure added it in d2b2f08, i will also add this requirement as a note in ansys sphinx theme docs

@Revathyvenugopal162 Revathyvenugopal162 merged commit c9169fb into main Aug 26, 2024
15 of 16 checks passed
@Revathyvenugopal162 Revathyvenugopal162 deleted the feat/add-quarto-pyansys-actions branch August 26, 2024 14:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement General improvements to existing features
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants