Skip to content

Install graphviz docs environment.yml #274

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

Merged

Conversation

mcbarton
Copy link
Collaborator

@mcbarton mcbarton commented Mar 6, 2025

Description

Please include a summary of changes, motivation and context for this PR.

If you visit the xeus-cpp read the docs build log here https://readthedocs.org/projects/xeus-cpp/builds/27398561/ you'll see many errors about dot not being found and not being able to do certain things. This is fixed if you install Graphviz like it mentions later on in the logs.

Fixes # (issue)

Type of change

Please tick all options which are relevant.

  • Bug fix
  • New feature
  • Added/removed dependencies
  • Required documentation updates

@codecov-commenter
Copy link

codecov-commenter commented Mar 6, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 80.56%. Comparing base (48767d6) to head (5acef3e).

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #274   +/-   ##
=======================================
  Coverage   80.56%   80.56%           
=======================================
  Files          20       20           
  Lines         957      957           
  Branches       88       88           
=======================================
  Hits          771      771           
  Misses        186      186           
🚀 New features to boost your workflow:
  • Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@mcbarton mcbarton force-pushed the Install-graphviz-doc-environment branch from d49e3d9 to 5acef3e Compare March 9, 2025 21:03
@mcbarton mcbarton requested a review from JohanMabille March 9, 2025 21:03
@mcbarton
Copy link
Collaborator Author

@anutosh491 @JohanMabille @vgvassilev pinging for review.

Copy link
Contributor

@vgvassilev vgvassilev left a comment

Choose a reason for hiding this comment

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

LGTM, however, I am wondering if there is some way to say this is a dependency only for the documentation -- I do not see how but would have been good to be able to express that...

@mcbarton
Copy link
Collaborator Author

mcbarton commented Apr 9, 2025

LGTM, however, I am wondering if there is some way to say this is a dependency only for the documentation -- I do not see how but would have been good to be able to express that...

This is only added as a dependency of the documentation already. You cannot build the documentation as part of the cmake build process. If you look at the file I edited you'll see that it is a environment file just for the docs folder.

@mcbarton mcbarton requested a review from vgvassilev April 9, 2025 09:17
@mcbarton
Copy link
Collaborator Author

LGTM, however, I am wondering if there is some way to say this is a dependency only for the documentation -- I do not see how but would have been good to be able to express that...

Merging, as been approved, and the question about adding as a documentation dependency is what we are already doing.

@mcbarton mcbarton merged commit d5cb124 into compiler-research:main Apr 13, 2025
14 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