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

Detect conda environments more reliably #736

Merged
merged 4 commits into from
Mar 29, 2021
Merged

Conversation

joaander
Copy link
Member

Description

Detect if linking to tbb in a conda environment by checking for the conda-meta directory relative to the found TBB library.

Motivation and Context

Fix readthedocs builds. Also support conda environments installed via other means (i.e. conda create --prefix).

Resolves: #735

How Has This Been Tested?

Will check the RTD PR build and validate that the docs build.

Screenshots

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds or improves functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation improvement (updates to user guides, docstrings, or developer docs)

Checklist:

  • I have read the CONTRIBUTING document.
  • My code follows the code style of this project.
  • I have updated the documentation (if relevant).
  • I have added tests that cover my changes (if relevant).
  • All new and existing tests passed.
  • I have updated the credits.
  • I have updated the Changelog.

@joaander joaander requested a review from a team as a code owner March 25, 2021 23:23
@codecov
Copy link

codecov bot commented Mar 26, 2021

Codecov Report

Merging #736 (860fde7) into master (8151715) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #736   +/-   ##
=======================================
  Coverage   55.32%   55.32%           
=======================================
  Files          16       16           
  Lines        2404     2404           
  Branches       35       35           
=======================================
  Hits         1330     1330           
  Misses       1061     1061           
  Partials       13       13           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8151715...860fde7. Read the comment docs.

@tommy-waltmann
Copy link
Collaborator

How do we check that this won't break like the previous docs did?

@bdice
Copy link
Member

bdice commented Mar 26, 2021

How do we check that this won't break like the previous docs did?

I don’t know of a way to make ReadTheDocs fail when the docs can’t be rendered in this way.

The release process I follow has a manual step for ensuring that ReadTheDocs is rendering, and I forgot / skipped that step.

@tommy-waltmann
Copy link
Collaborator

Ok, if we follow that step this time, then we should be fine. I just wanted to make sure that whatever slipped under our nose last time wasn't going to happen again.

@bdice
Copy link
Member

bdice commented Mar 26, 2021

Ok, if we follow that step this time, then we should be fine. I just wanted to make sure that whatever slipped under our nose last time wasn't going to happen again.

The best thing to do would be to create a pull request template for releases that includes all this information as a checklist. I’ll create an issue for that. (Edit: #737)

@joaander
Copy link
Member Author

There is a fail_on_warning setting: https://docs.readthedocs.io/en/stable/config-file/v2.html#sphinx . I think if that were enabled, the rtd pull request task would have failed.

@tommy-waltmann tommy-waltmann merged commit b70318c into master Mar 29, 2021
@tommy-waltmann tommy-waltmann deleted the fix-rtd-builds branch March 29, 2021 15:17
@tommy-waltmann tommy-waltmann added this to the v2.5.1 milestone Apr 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Freud API docs aren't loading on readthedocs.io
3 participants