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

Switch RTD build to mamba #126

Merged
merged 4 commits into from Jan 12, 2022
Merged

Switch RTD build to mamba #126

merged 4 commits into from Jan 12, 2022

Conversation

jbusecke
Copy link
Collaborator

@jbusecke jbusecke commented Jan 5, 2022

Towards addressing problems in #120

@codecov-commenter
Copy link

codecov-commenter commented Jan 5, 2022

Codecov Report

Merging #126 (0210d56) into master (75f172c) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #126   +/-   ##
=======================================
  Coverage   98.41%   98.41%           
=======================================
  Files           9        9           
  Lines         944      944           
=======================================
  Hits          929      929           
  Misses         15       15           
Flag Coverage Δ
unittests 98.41% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.


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 75f172c...0210d56. Read the comment docs.

@jbusecke
Copy link
Collaborator Author

jbusecke commented Jan 6, 2022

So if I comment out these lines the build succeeds. I am kind of confused why this would have not been an issue before switching to mamba.

Apparently I put it in there a while ago, but if the docs look OK I would just remove it?

Could someone go through the logs and the docs created for this PR and double check if the warnings can be ignored and if the docs look ok?

@NoraLoose
Copy link
Member

Thanks for this PR @jbusecke! I checked the docs, and they look ok. I'm not sure about the warnings in the logs. Probably ok to ignore them(?)

@jbusecke
Copy link
Collaborator Author

I would say so for now. Since its not part of the actual code, the docs output is all that matters, right?

@NoraLoose
Copy link
Member

Yes. I think we can merge this!

Would you have time to work on #120 with me next? It would be great to get your input there!

@jbusecke
Copy link
Collaborator Author

Quite tight on time today but I can definitely take a look!

@NoraLoose
Copy link
Member

Thanks! I didn't mean today, just whenever you have a chance. Just asking because from what you wrote in #120 you think that mamba can help over there. :)

@NoraLoose NoraLoose merged commit 4e8cb78 into master Jan 12, 2022
@jbusecke
Copy link
Collaborator Author

Oh yes absolutely! Sorry for the misinterpretation.

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.

None yet

3 participants