aiidaplugins / aiida-lammps

LAMMPS plugin for AiiDA
https://aiida-lammps.readthedocs.io
MIT License
25 stars 14 forks source link

:books: docs documentation overhaul #86

Closed JPchico closed 11 months ago

JPchico commented 11 months ago

(WIP) Improvements to the documentation before the release.

JPchico commented 11 months ago

Hi @chrisjsewell and @sphuber I have finally had some time to work on the documentation. It is a work in progress and I plan to be making more changes and additions in the coming days.

I drew quite heavily from the QE plugin structure and formatting as I think it is the most complete right now.

Let me know if you think that the structure makes sense and I'll keep updating regularly.

codecov[bot] commented 11 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Comparison is base (77f4594) 85.30% compared to head (c44a673) 85.30%.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## develop #86 +/- ## ======================================== Coverage 85.30% 85.30% ======================================== Files 19 19 Lines 1599 1599 ======================================== Hits 1364 1364 Misses 235 235 ``` | [Flag](https://app.codecov.io/gh/aiidaplugins/aiida-lammps/pull/86/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=aiidaplugins) | Coverage Δ | | |---|---|---| | [pytests](https://app.codecov.io/gh/aiidaplugins/aiida-lammps/pull/86/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=aiidaplugins) | `85.30% <ø> (ø)` | | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=aiidaplugins#carryforward-flags-in-the-pull-request-comment) to find out more.

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

sphuber commented 11 months ago

Thanks a lot @JPchico . Unfortunately, since the docs build fails (maybe just warnings) I cannot see the rendered version easily. Could you try addressing the failure. It would be much easier to review by looking at the built docs.

chrisjsewell commented 11 months ago

Could you try addressing the failure.

Yeh looks like you need to fix some of the toctree

pickling environment... /home/docs/checkouts/readthedocs.org/user_builds/aiida-lammps/checkouts/86/docs/source/index.md:25: WARNING: toctree contains reference to nonexisting document 'reference/api/index'
done
checking consistency... /home/docs/checkouts/readthedocs.org/user_builds/aiida-lammps/checkouts/86/docs/source/developers/intro.md: WARNING: document isn't included in any toctree
/home/docs/checkouts/readthedocs.org/user_builds/aiida-lammps/checkouts/86/docs/source/reference/api/auto/aiida_lammps/calculations/index.rst: WARNING: document isn't included in any toctree
/home/docs/checkouts/readthedocs.org/user_builds/aiida-lammps/checkouts/86/docs/source/reference/api/auto/aiida_lammps/data/index.rst: WARNING: document isn't included in any toctree
/home/docs/checkouts/readthedocs.org/user_builds/aiida-lammps/checkouts/86/docs/source/reference/api/auto/aiida_lammps/parsers/index.rst: WARNING: document isn't included in any toctree
/home/docs/checkouts/readthedocs.org/user_builds/aiida-lammps/checkouts/86/docs/source/reference/api/auto/aiida_lammps/utils/index.rst: WARNING: document isn't included in any toctree
/home/docs/checkouts/readthedocs.org/user_builds/aiida-lammps/checkouts/86/docs/source/reference/api/auto/aiida_lammps/validation/index.rst: WARNING: document isn't included in any toctree
/home/docs/checkouts/readthedocs.org/user_builds/aiida-lammps/checkouts/86/docs/source/reference/api/auto/aiida_lammps/workflows/index.rst: WARNING: document isn't included in any toctree
/home/docs/checkouts/readthedocs.org/user_builds/aiida-lammps/checkouts/86/docs/source/users/example_md.md: WARNING: document isn't included in any toctree
/home/docs/checkouts/readthedocs.org/user_builds/aiida-lammps/checkouts/86/docs/source/users/example_minimize.md: WARNING: document isn't included in any toctree
/home/docs/checkouts/readthedocs.org/user_builds/aiida-lammps/checkouts/86/docs/source/users/get_started.md: WARNING: document isn't included in any toctree
JPchico commented 11 months ago

Hi! sorry, I did not notice that the building was failing in the CI since it was working locally. I think it should be fixed now.

JPchico commented 11 months ago

@sphuber and @chrisjsewell I have fixed the issues with the build and I have added a bit more of information. There is still more work needed but I think that it is getting there.

I also changed some of the examples to use the requests library to download the files needed, like the data.rhodo for the raw calculation.

JPchico commented 11 months ago

@sphuber and @chrisjsewell I have made the suggested changes and added a bit more on the workflows and calculations topics. I think that this is a quite good stage for the documentation right now. If you agree I will merge and close the issue.

After that I will deal with two small things that I realized while dealing with this (will open issues on those now) and I think that then we can make the release. I have never made a PyPI release, but for what I see it should automatically done when the release is done in the github side. Is that correct?

sphuber commented 11 months ago

Yep, the cd.yml workflow should take care of that. All you have to do is make a commit that bumps the version number (and update the CHANGELOG.md), tag it and then push the tag. The pushed tag will trigger the workflow and if all tests/pre-commit pass, it will build and push to PyPI.

sphuber commented 11 months ago

I would also propose that after that is done, we simply merge develop into master and just use master as the main development branch and get rid of develop. We have been using this approach in all other aiidateam and aiidaplugins repos and it works perfectly fine. We really don't have the need for a two branch approach.