ukaea / paramak

Create parametric 3D fusion reactor CAD models
https://paramak.readthedocs.io/en/main/
37 stars 12 forks source link

added include readme + m2r2 for conversion #807

Closed RemDelaporteMathurin closed 3 years ago

RemDelaporteMathurin commented 3 years ago

Proposed changes

This PR refactores the docs by directly including the README.md file in the docs/source/index.rst file.

This greatly improves the code maintainability by reducing "code" duplication.

Types of changes

What types of changes does your code introduce to the Paramak? Put an x in the boxes that apply

Checklist

Put an x in the boxes that apply. You can also fill these out after creating the PR. If you're unsure about any of them, don't hesitate to ask. We're here to help! This is simply a reminder of what we are going to look for before merging your code.

Further comments

Only "drawback" is it now shows all the badges in the README page of the docs (but maybe that's not too much of an issue?) What do you think @Shimwell @billingsley-john ?

RemDelaporteMathurin commented 3 years ago

Failing tests related to #806

shimwell commented 3 years ago

Great suggestion, I really like it

shimwell commented 3 years ago

Looking at the new docs that were built we have a few things to consider.

The readme contains a like to the documentation, so do this PR would mean the documentation contains a like to itself. The readme has no list of contents with links The readme.md is missing the YouTube video.

The readme has lots of duplication in but perhaps the way to solve this is to remove the duplication from the readme instead of mirroring the readme in the read the docs

Perhaps as these two things are different it is best just to reduce the readme to a bare minimum and have a big link to the documentation

shimwell commented 3 years ago

Also just to mention the readme is also copied here https://pypi.org/project/paramak/

codecov[bot] commented 3 years ago

Codecov Report

Merging #807 (5133b8d) into develop (ed33d62) will not change coverage. The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff            @@
##           develop     #807   +/-   ##
========================================
  Coverage    95.43%   95.43%           
========================================
  Files           74       74           
  Lines         5252     5252           
========================================
  Hits          5012     5012           
  Misses         240      240           

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 ed33d62...5133b8d. Read the comment docs.

RemDelaporteMathurin commented 3 years ago

The readme contains a like to the documentation, so do this PR would mean the documentation contains a like to itself.

Yes. Doception

The readme has no list of contents with links

The list of content still appears at the bottom of the page (we can make it appear before the readme).

The readme.md is missing the YouTube video.

Yep but it has a link

The readme has lots of duplication in but perhaps the way to solve this is to remove the duplication from the readme instead of mirroring the readme in the read the docs

Perhaps as these two things are different it is best just to reduce the readme to a bare minimum and have a big link to the documentation

As you like, it was just an idea to reduce the amount of maintainance when changes are made in the doc 😃

shimwell commented 3 years ago

Perhaps #814 is another option

RemDelaporteMathurin commented 3 years ago

Perhaps #814 is another option

I think that one is better. CLosing this :-)