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

[docs] Use of mdinclude directive to add .md files to sphinx docs #291

Merged
merged 2 commits into from
Jun 28, 2020

Conversation

rpanderson
Copy link
Contributor

@rpanderson rpanderson commented Jun 28, 2020

Description

Partially resolves #275.

Proposed Changes

Use the mdinclude directive of m2r, per readthedocs/recommonmark#191 (comment) for including benchmarks/ecg_preprocessing/README.md on the Benchmarks page. To make this work, I had to additionally:

  • Use HTML for any hyperlinks in bold/italics;
  • Remove blank lines from references div; and
  • Relative link figures from docs directory, i.e. prepend ../../benchmarks/ecg_preprocessing to image paths.

The latter does not break images on the modified README when viewed on GitHub as the relative link coincidentally resolves the same file from both locations.

Checklist

  • I have read the CONTRIBUTING file.
  • My PR is targetted at the dev branch (and not towards the master branch).
  • I ran the CODE CHECKS on the files I added or modified and fixed the errors found isort, black, and flake8 failed for reasons unrelated to my modifications. Happy to fix but not including in this PR initially.

Use m2r for the mdinclude directive and recommonmark for everything else, per readthedocs/recommonmark#191 (comment). To make this work, I had to additionally:
* Use HTML for any hyperlinks in bold/italics;
* Relative link figures from docs directory, i.e. prepend ../../benchmarks/ecg_preprocessing to image paths; and
* Remove line blank lines from references div.
@rpanderson rpanderson closed this Jun 28, 2020
@rpanderson rpanderson reopened this Jun 28, 2020
@rpanderson
Copy link
Contributor Author

Sorry, @DominiqueMakowski; I neglected to target dev branch when opening this PR (I did originally but didn't when re-composing the PR after closing my browser tab). Could you please change the base branch? Also, is there any reason you haven't set dev as your default branch?

@DominiqueMakowski DominiqueMakowski changed the base branch from master to dev June 28, 2020 14:24
@DominiqueMakowski
Copy link
Member

Could you please change the base branch?

I didn't even know I could do that, thanks a lot haha!

Also, is there any reason you haven't set dev as your default branch?

I don't know (GitHub isn't my strength ^^), would it affect the repo in any other way?

Before I merge, please consider adding yourself to the contributors ☺️

@DominiqueMakowski DominiqueMakowski changed the title Example use of mdinclude directive for benchmarks/ecg_preprocessing [docs] Use of mdinclude directive to add .md files to sphinx docs Jun 28, 2020
@codecov-commenter
Copy link

codecov-commenter commented Jun 28, 2020

Codecov Report

Merging #291 into dev will increase coverage by 0.05%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##              dev     #291      +/-   ##
==========================================
+ Coverage   85.45%   85.51%   +0.05%     
==========================================
  Files         156      156              
  Lines        7116     7116              
==========================================
+ Hits         6081     6085       +4     
+ Misses       1035     1031       -4     
Impacted Files Coverage Δ
neurokit2/rsp/rsp_simulate.py 99.10% <0.00%> (+3.57%) ⬆️

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 b2410d1...a90f157. Read the comment docs.

@rpanderson
Copy link
Contributor Author

I didn't even know I could do that, thanks a lot haha!

Neither did I until I made the targetted the wrong branch!

Also, is there any reason you haven't set dev as your default branch?

I don't know (GitHub isn't my strength ^^), would it affect the repo in any other way?

I'm not an authority on this, but it doesn't seem to (aside from affecting git ls-remote). Discussion on the convention abounds (one example of many), but it solves the problem of (from the linked issue) "opening PRs against the wrong branch like I did."

@DominiqueMakowski
Copy link
Member

Indeed, from the discussion, it seems that it would make sense for us to set dev as the default, especially since the last release we try to adopt a feature-PR -> dev workflow and to merge to dev only once a feature is "stable", so I reckon dev is stable enough so that it could work as a default. I'll definitely consider that

Anyway, thanks again for your help!! Merging now and welcome onboard 😁

@DominiqueMakowski DominiqueMakowski merged commit b2344a2 into neuropsychology:dev Jun 28, 2020
@rpanderson rpanderson deleted the mdinclude-example branch June 29, 2020 04:24
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.

Enable Markdown files to be added to the Sphinx docs
3 participants