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 read the docs build with automatic versioning #182

Merged
merged 13 commits into from
Mar 11, 2021
Merged

Conversation

JessicaS11
Copy link
Member

An alternative to #181 (as suggested there), this adds the setuptools method to the readthedocs.yml and successfully builds on read the docs.

@lsetiawan
Copy link
Contributor

@JessicaS11 I guess if there's already a readthedocs automated test, then we can just remove the github workflow one!

@lsetiawan
Copy link
Contributor

Thanks for figuring all this out! Sorry I wasn't much help here haha Not super familiar with readthedocs and setuptools_scm! 😛

@JessicaS11
Copy link
Member Author

@JessicaS11 I guess if there's already a readthedocs automated test, then we can just remove the github workflow one!

Facepalm. Good point. Okay... here come the edits.

@weiji14
Copy link
Member

weiji14 commented Mar 11, 2021

@JessicaS11 I guess if there's already a readthedocs automated test, then we can just remove the github workflow one!

Was just about to say this too!

Thanks for figuring all this out! Sorry I wasn't much help here haha Not super familiar with readthedocs and setuptools_scm! stuck_out_tongue

Don't worry Don, I don't think many of us are good with it too, we just keep trying stuff until it works pretty much!

@codecov-io
Copy link

codecov-io commented Mar 11, 2021

Codecov Report

Merging #182 (71c1242) into development (08546a0) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff              @@
##           development     #182   +/-   ##
============================================
  Coverage        48.17%   48.17%           
============================================
  Files               17       17           
  Lines             1179     1179           
  Branches           262      262           
============================================
  Hits               568      568           
  Misses             570      570           
  Partials            41       41           

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 08546a0...71c1242. Read the comment docs.

Comment on lines 25 to 31
# -- Versioning with setuptools_scm ------------------------------------------
from pkg_resources import get_distribution
release = get_distribution('icepyx').version
# for example take major/minor
version = '.'.join(release.split('.')[:2])


Copy link
Member Author

Choose a reason for hiding this comment

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

So do we need this still? Or can we get rid of it?

Copy link
Member

Choose a reason for hiding this comment

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

Good question, I tried deleting these lines locally and running cd icepyx/doc && make html works. Could you try and remove them and see if the readthedocs build passes still?

@weiji14
Copy link
Member

weiji14 commented Mar 11, 2021

Probably best to revert the dev .ipynb notebooks removal too to keep this PR clean:

git revert 7a56f48dff896d6a997f4a2290b1b50e67a44399

Copy link
Member

@weiji14 weiji14 left a comment

Choose a reason for hiding this comment

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

Amazing what 2 lines of code can do! Great job team!

@JessicaS11 JessicaS11 merged commit 7458296 into development Mar 11, 2021
@JessicaS11 JessicaS11 deleted the fix_rtd branch March 11, 2021 22:30
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.

4 participants