MRChemSoft / mrcpp

MultiResolution Computation Program Package
GNU Lesser General Public License v3.0
12 stars 19 forks source link

MWTree and MWNode documentation #219

Closed ilfreddy closed 1 year ago

ilfreddy commented 1 year ago

MWTree and MWNode classes are now sufficiently documented.

Workflow to approve this pull request (and all other PRs about documentation)

  1. pull the current branch
  2. push it to the rtd-build in master
  3. check that the commit does not alter anything else other than the documentation
  4. check that the documentation produced is sane, meaningful and well written (language, grammar, typos)
  5. check that all tests pass as requested
  6. approve :-)

NOTE: This PR includes quite a few more changes other than just in the MWNode and MWTree classes because we had to refresh a few config files.

robertodr commented 1 year ago

If anyone with access can make me admin for the documentation project on readthedocs.org, I can activate the documentation build on pull request. It will be easier to review changes such as these

ilfreddy commented 1 year ago

I don't seem to be admin for mrcpp on readthedocs. Maybe @stigrj?

Gabrielgerez commented 1 year ago

If anyone with access can make me admin for the documentation project on readthedocs.org, I can activate the documentation build on pull request. It will be easier to review changes such as these

I thought i enabled that already, but it is weird that it did not trigger on this PR. Is it not enough to just enable it in the advanced settings > Build pull requests for this project ?

robertodr commented 1 year ago

According to the docs, yes, that's enough. Investigating...

robertodr commented 1 year ago

The build starts, but then fails...

codecov[bot] commented 1 year ago

Codecov Report

All modified lines are covered by tests :white_check_mark:

Comparison is base (5320eb4) 66.48% compared to head (e5262aa) 66.48%.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #219 +/- ## ======================================= Coverage 66.48% 66.48% ======================================= Files 169 169 Lines 9810 9811 +1 ======================================= + Hits 6522 6523 +1 Misses 3288 3288 ``` | [Files](https://app.codecov.io/gh/MRChemSoft/mrcpp/pull/219?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=MRChemSoft) | Coverage Δ | | |---|---|---| | [src/operators/IdentityConvolution.cpp](https://app.codecov.io/gh/MRChemSoft/mrcpp/pull/219?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=MRChemSoft#diff-c3JjL29wZXJhdG9ycy9JZGVudGl0eUNvbnZvbHV0aW9uLmNwcA==) | `100.00% <ø> (ø)` | | | [src/operators/MWOperator.h](https://app.codecov.io/gh/MRChemSoft/mrcpp/pull/219?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=MRChemSoft#diff-c3JjL29wZXJhdG9ycy9NV09wZXJhdG9yLmg=) | `75.00% <ø> (ø)` | | | [src/trees/MWNode.cpp](https://app.codecov.io/gh/MRChemSoft/mrcpp/pull/219?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=MRChemSoft#diff-c3JjL3RyZWVzL01XTm9kZS5jcHA=) | `80.61% <ø> (-0.04%)` | :arrow_down: | | [src/trees/MWNode.h](https://app.codecov.io/gh/MRChemSoft/mrcpp/pull/219?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=MRChemSoft#diff-c3JjL3RyZWVzL01XTm9kZS5o) | `71.15% <ø> (ø)` | | | [src/trees/MWTree.cpp](https://app.codecov.io/gh/MRChemSoft/mrcpp/pull/219?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=MRChemSoft#diff-c3JjL3RyZWVzL01XVHJlZS5jcHA=) | `79.25% <100.00%> (ø)` | | | [src/trees/MWTree.h](https://app.codecov.io/gh/MRChemSoft/mrcpp/pull/219?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=MRChemSoft#diff-c3JjL3RyZWVzL01XVHJlZS5o) | `72.41% <ø> (ø)` | | ... and [2 files with indirect coverage changes](https://app.codecov.io/gh/MRChemSoft/mrcpp/pull/219/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=MRChemSoft)

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

robertodr commented 1 year ago

Seems it needed a re-sync of the github hook. Now it shows the documentation build in the list of PR checks. It's also building the docs.

robertodr commented 1 year ago

Seems you need to debug the build on readthedocs before we can merge this. https://readthedocs.org/projects/mrcpp/builds/22034460/

ilfreddy commented 1 year ago

I don't quite get it. The last time I commited the build on readthedocs (which i triggered manually by pushing to the rtd-build branch) was working just fine. So something must have happened in the meantime...

Gabrielgerez commented 1 year ago

Seems it needed a re-sync of the github hook. Now it shows the documentation build in the list of PR checks. It's also building the docs.

How did you do this?

I don't quite get it. The last time I commited the build on readthedocs (which i triggered manually by pushing to the rtd-build branch) was working just fine. So something must have happened in the meantime...

did you do a rebase before creating the PR? I think it might be some other issues that are crashing though

ilfreddy commented 1 year ago

Not clear to me what happened but the doc build seems fine now after I force-pushed my own version

ilfreddy commented 1 year ago

This PR is btw from the rtd-build which is on upstream (not my own fork, my bad!).

robertodr commented 1 year ago

How did you do this?

By reading the docs 😄 https://docs.readthedocs.io/en/stable/guides/pull-requests.html#troubleshooting