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 sys.path adjustment in doc config #734

Merged
merged 2 commits into from
Sep 21, 2021
Merged

Fix sys.path adjustment in doc config #734

merged 2 commits into from
Sep 21, 2021

Conversation

michaeljones
Copy link
Collaborator

It seems that CI already changes the PYTHONPATH to adjust for this but
this allows us to run 'make html' in either the root or 'documentation'
and have it work as it used to.

@vermeeren
Copy link
Collaborator

@michaeljones This seems fine to me, any reason it's not merged yet assuming you have confirmed it works?

@jakobandersen
Copy link
Collaborator

After this, is the PYTHONPATH adjustment at https://github.com/michaeljones/breathe/blob/master/.github/workflows/documentation.yml#L27 then needed? Otherwise I suggest removing it so the CI is closer to what we do manually.

It seems that CI already changes the PYTHONPATH to adjust for this but
this allows us to run 'make html' in either the root or 'documentation'
and have it work as it used to.
Hoping that the adjustment to the sphinx conf file is sufficient.
@michaeljones
Copy link
Collaborator Author

I've rebased & adjust the test file to remove the PYTHONPATH manipulation to see if that still works.

I think I've been used to waiting for approvals on PRs from work for a while now and now I don't know how needs to (if anyone) approve such things. I will probably relax with time and start merging things that seem safe without waiting so long :)

@michaeljones
Copy link
Collaborator Author

Seems to have passed :)

@michaeljones michaeljones merged commit 436c4f7 into master Sep 21, 2021
@michaeljones michaeljones deleted the fix-sys-path branch September 27, 2021 17:42
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