Skip to content
This repository has been archived by the owner on Dec 5, 2024. It is now read-only.

Rtd warning fix #100

Merged
merged 12 commits into from
Nov 30, 2021
Merged

Conversation

NishanthJKumar
Copy link
Contributor

Should fix warnings and issues with the RTD build!

@codecov-commenter
Copy link

codecov-commenter commented Nov 30, 2021

Codecov Report

Merging #100 (628b1fb) into master (1f9633b) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##            master      #100   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            8         8           
  Lines          633       633           
=========================================
  Hits           633       633           

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 1f9633b...628b1fb. Read the comment docs.

@StannisZhou
Copy link
Contributor

Could you also update

python-version: [3.7]
so that tests are also run for python 3.8?

Also, should we change this fail on warning behavior

fail_on_warning: true
so that CI blocks when docs building actually fails (not sure if this is the argument controlling this behavior)?

@NishanthJKumar
Copy link
Contributor Author

So for the first point, I'm not sure if we want to test both 3.7 and 3.8 (it'll double the time taken for running CI). I'm happy to put it in if you'd like, but maybe we can just test 3.8 (because we'd expect it to work on 3.7 if it works on 3.8)?

For the second part, this flag is saying that if there are any warnings, then RTD will fail (if there are errors, it will also fail by default). I'll update CI to make it fail on warnings!

Copy link
Contributor

@StannisZhou StannisZhou left a comment

Choose a reason for hiding this comment

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

LGTM

OK let's keep CI testing for only python 3.7 for now. But it would be good if we at least do one test somewhere to make sure all tests pass for python 3.8. Maybe you can manually kick off a run on a branch of yours with python 3.8? Just want to make sure nothing breaks unexpectedly.

@NishanthJKumar
Copy link
Contributor Author

Sounds good! I ran tests locally with a python 3.8 env and everything worked!

@NishanthJKumar NishanthJKumar merged commit 2dec84f into vicariousinc:master Nov 30, 2021
@NishanthJKumar NishanthJKumar deleted the rtd_warning_fix branch November 30, 2021 06:18
@NishanthJKumar NishanthJKumar linked an issue Nov 30, 2021 that may be closed by this pull request
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Verify RTD integration and do more setup once repo is public
3 participants