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 from docstr-coverage package to interrogate in CI pipelines #2027

Merged
merged 1 commit into from May 27, 2022

Conversation

epassaro
Copy link
Member

@epassaro epassaro commented May 16, 2022

Description

Use the interrogate package instead of docstr-coverage. This package retrieves the docstring coverage in a nice and easy-to-read table.

Motivation and context

The docstr-coverage has a long output and it's difficult to follow.

How has this been tested?

  • Testing pipeline
  • Other

The action should fail in this pull request because the helper script does not exist yet in the master branch. Also, should fail the first time after merging this pull request for the same reason (the helper script does not exist in the previous commit).

A working example on my fork: https://github.com/epassaro/tardis/runs/6426582254?check_suite_focus=true#step:7:1

Examples

Type of change

  • Bug fix
  • New feature
  • Breaking change
  • None of the above

Checklist

  • I have updated the documentation according to my changes
  • I have built the documentation by applying the build_docs label to this pull request (if you don't have enough privileges a reviewer will do it for you)
  • I have requested two reviewers for this pull request

@epassaro epassaro changed the title Switch from docstr-coverage to interrogate in CI Switch from docstr-coverage package to interrogate in CI pipelines May 16, 2022
omit-covered-files = false
quiet = false
verbose = 0
Copy link
Contributor

Choose a reason for hiding this comment

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

I assume this matches our existing coverage settings?

Copy link
Member Author

Choose a reason for hiding this comment

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

Good question. Our previous configuration was:

paths:
  - tardis
#badge: docs
#exclude: .*/test # regex
verbose: 2 # int (0-3)
skip_magic: True
skip_file_doc: True
skip_init: True
skip_class_def: False
skip_private: True
follow_links: True
#ignore_names_file: .*/test # regex
#fail_under: 90
percentage_only: False
#ignore_patterns: # Dict with key/value pairs of file-pattern/node-pattern
#  .*: method_to_ignore_in_all_files
#  FileWhereWeWantToIgnoreAllSpecialMethods: "__.+__"
#  SomeFile:
#    - method_to_ignore1
#    - method_to_ignore2
#    - method_to_ignore3
#  a_very_important_view_file:
#    - "^get$"
#    - "^set$"
#    - "^post$"
#  detect_.*:
#    - "get_val.*"

Seems quite similar.

The actual coverage with interrogate is 50.08 and the previous one is 49.65. What's your opinion on this?

Copy link
Contributor

Choose a reason for hiding this comment

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

Close enough for me!

@codecov
Copy link

codecov bot commented May 16, 2022

Codecov Report

Merging #2027 (3e70520) into master (0ece835) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #2027   +/-   ##
=======================================
  Coverage   60.13%   60.13%           
=======================================
  Files          70       70           
  Lines        8108     8108           
=======================================
  Hits         4876     4876           
  Misses       3232     3232           

📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more

@andrewfullard
Copy link
Contributor

Why is the docstr-cov failing with an interrogate error?

@epassaro
Copy link
Member Author

Why is the docstr-cov failing with an interrogate error?

It's described in the PR body:

The action should fail in this pull request because the helper script does not exist yet in the master branch. Also, should fail the first time after merging this pull request for the same reason (the helper script does not exist in the previous commit).

A working example on my fork: https://github.com/epassaro/tardis/runs/6426582254?check_suite_focus=true#step:7:1

@epassaro epassaro merged commit 9d10e6e into tardis-sn:master May 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants