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

Housekeeping #277

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Housekeeping #277

wants to merge 4 commits into from

Conversation

avylove
Copy link
Collaborator

@avylove avylove commented Nov 23, 2024

A few housekeeping tasks

  • Fix pylint errors that were breaking CI tests
    • Too many positional arguments on Termcap.build()
  • Fix Sphinx issues that were breaking CI tests
    • Deprecated behavior in conf.py
    • csv-table used for keycodes now requires the deliminator in the header rather than a comma
  • Update Python versions
    • Default for testing switched to 3.13
    • Added 3.14-pre as optional in CI
  • Added logic to CI to use Codecov uploader version 4 except for Python 2.7, which uses version 3
    • Maybe this will help with the random rate limit errors
    • They released version 5 on November 14th, but it might be good to wait since they have done 7 bug releases since then

Copy link

codecov bot commented Nov 23, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 95.31%. Comparing base (167c34e) to head (57f6d5f).

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #277   +/-   ##
=======================================
  Coverage   95.31%   95.31%           
=======================================
  Files           9        9           
  Lines        1025     1025           
  Branches      216      177   -39     
=======================================
  Hits          977      977           
  Misses         44       44           
  Partials        4        4           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.


🚨 Try these New Features:

@@ -155,7 +154,7 @@ def wrapper(decorator):
# html_theme_options = {}

# Add any paths that contain custom themes here, relative to this directory.
html_theme_path = [sphinx_rtd_theme.get_html_theme_path()]
# html_theme_path = []
Copy link
Owner

@jquast jquast Dec 10, 2024

Choose a reason for hiding this comment

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

You don't mention why to remove this, I think I put it here to match local development to the theme where it is published, in any case it uses a requirement "sphinx_rtd_theme" in docs/requirements.txt, so then that requirement should also be removed.

Sorry to be late, I've been busy with school work but that's all over tomorrow

@jquast jquast self-requested a review December 10, 2024 21:44
Copy link
Owner

@jquast jquast left a comment

Choose a reason for hiding this comment

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

approved -- but maybe you want to delete sphinx_rtd_theme requirement ?

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.

2 participants