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

Cleaning up docs build #1801

Merged
merged 5 commits into from
Oct 25, 2021
Merged

Conversation

isaacgsmith
Copy link
Member

Description

  • Generates ZENODO.rst by hooking into sphinx.
  • Adds generated files to .gitignore.
  • Formatting conf.py with black.

Motivation and context

Makes sure TARDIS is following best practices. See the discussion on #1757 for more context.

How has this been tested?

  • Testing pipeline.
  • Other.
    Docs built locally and on github.

Examples

https://smithis7.github.io/tardis/branch/conf_format_doc/index.html

Type of change

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

Checklist

  • My change requires a change to the documentation.
    • I have updated the documentation accordingly.
    • (optional) I have built the documentation on my fork following the instructions.
  • I have assigned and requested two reviewers for this pull request.

@tardis-bot
Copy link
Contributor

Before a pull request is accepted, it must meet the following criteria:

  • Is the necessary information provided?
  • Is this a duplicate PR?
    • If a new PR is clearly a duplicate, ask how this PR is different from the original PR?
    • If this PR is about to be merged, close the original PR with a link to this new PR that solved the issue.
  • Does it pass existing tests and are new tests provided if required?
    • The test coverage should not decrease, and for new features should be close to 100%.
  • Is the code tidy?
    • No unnecessary print lines or code comments.

@sourcery-ai
Copy link

sourcery-ai bot commented Sep 27, 2021

Sourcery Code Quality Report

✅  Merging this PR will increase code quality in the affected files by 0.79%.

Quality metrics Before After Change
Complexity 3.42 ⭐ 3.36 ⭐ -0.06 👍
Method Length 98.43 🙂 87.62 🙂 -10.81 👍
Working memory 9.34 🙂 9.13 🙂 -0.21 👍
Quality 59.04% 🙂 59.83% 🙂 0.79% 👍
Other metrics Before After Change
Lines 374 397 23
Changed files Quality Before Quality After Quality Change
docs/conf.py 59.04% 🙂 59.83% 🙂 0.79% 👍

Here are some functions in these files that still need a tune-up:

File Function Complexity Length Working Memory Quality Recommendation
docs/conf.py create_redirect_files 3 ⭐ 94 🙂 10 😞 66.84% 🙂 Extract out complex expressions

Legend and Explanation

The emojis denote the absolute quality of the code:

  • ⭐ excellent
  • 🙂 good
  • 😞 poor
  • ⛔ very poor

The 👍 and 👎 indicate whether the quality has improved or gotten worse with this pull request.


Please see our documentation here for details on how these metrics are calculated.

We are actively working on this report - lots more documentation and extra metrics to come!

Help us improve this quality report!

@codecov
Copy link

codecov bot commented Sep 27, 2021

Codecov Report

Merging #1801 (19eb1e5) into master (efebacd) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1801   +/-   ##
=======================================
  Coverage   58.25%   58.25%           
=======================================
  Files          66       66           
  Lines        6721     6721           
=======================================
  Hits         3915     3915           
  Misses       2806     2806           

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 efebacd...19eb1e5. Read the comment docs.

docs/conf.py Outdated Show resolved Hide resolved
@epassaro
Copy link
Member

We do not format conf.py or any other package template file. But in this case maybe it's worth since we use that file a lot, I don't know. But certainly not in this PR, black formatting should be done in a separate PR and then added to the .git-blame-ignore-revs.

@isaacgsmith
Copy link
Member Author

@epassaro sorry to make changes while you were reviewing. I had a docs failure so I just had to revert it back. I don't see the harm in doing the black formatting in this PR, I think it should be formatted correctly, and this seems as good of a PR as any to add that in, right?

@epassaro
Copy link
Member

@epassaro sorry to make changes while you were reviewing. I had a docs failure so I just had to revert it back. I don't see the harm in doing the black formatting in this PR, I think it should be formatted correctly, and this seems as good of a PR as any to add that in, right?

https://akrabat.com/ignoring-revisions-with-git-blame/

Copy link
Member

@jaladh-singhal jaladh-singhal left a comment

Choose a reason for hiding this comment

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

@smithis7 You've applied your organization skills excellently in code too - conf.py now looks a lot neater. Thanks for this PR other changes like gitignoring were required too!

I just have one small comment and then it's ready to be merged (do ping me when you push it so that I don't forget).

docs/conf.py Outdated Show resolved Hide resolved
docs/conf.py Show resolved Hide resolved
docs/conf.py Outdated Show resolved Hide resolved
Copy link
Member

@jaladh-singhal jaladh-singhal left a comment

Choose a reason for hiding this comment

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

Thanks for the update @smithis7 - ready to merge!

@andrewfullard andrewfullard merged commit 6dc581d into tardis-sn:master Oct 25, 2021
@isaacgsmith isaacgsmith deleted the conf_format_doc branch October 26, 2021 21:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants