Closed roche-emmanuel closed 3 months ago
Totals | |
---|---|
Change from base Build 10528275069: | 0.0% |
Covered Lines: | 52408 |
Relevant Lines: | 54506 |
Hi everyone,
In this PR, I have mainly focused on modifying the doc/source/dev_guide/CONTRIBUTING.rst
file. Initially, this was a simple symlink pointing to ../../../CONTRIBUTING.rst
. However, when checking out the Satpy repo on a Windows machine, this symlink gets converted into a file containing only ../../../CONTRIBUTING.rst
.
At this point, if you try to run the pre-commit checks, the end-of-file checker will add a \n
at the end of that "file," and you might then commit that change to the repo. However, for Git, this is still treated as a symlink, and it will append the \n
character to the link source path (at least, that has been my experience).
This behavior confuses the ReadTheDocs build workflow entirely, likely because it runs on Linux and attempts to read a link source such as ../../../CONTRIBUTING.rst\n
.
The solution I’ve implemented here is to avoid using a symlink altogether and instead rely on the RST include
directive to load the ../../../CONTRIBUTING.rst
file.
With this change, the ReadTheDocs build seems to work fine, and access to the CONTRIBUTING.rst
file also appears to be correct based on the workflow results on this page: https://satpy--2890.org.readthedocs.build/en/2890/dev_guide/index.html (clicking on the "How to contribute" links).
So, I’m now marking this PR as ready for review. Of course, please let me know if you think there’s anything that should be changed before it can be merged. Thanks 🙏!
Hi @djhoese ,
Thanks for your review and feedback on this PR.
Concerning your request for change above, I also think it's a good idea to keep all the sources for the doc generation self-contained in the doc/ folder.
Now, from a "github perspective", I think the /CONTRIBUTING.rst file is the most important one; and this is the target that will be used at different locations on github, for instance just at the bottom of this page ;-). And I would say that "contribution guidelines" are most of the time used by developers while doing some work: so there is also a good chance this /CONTRIBUTING.rst file will be accessed regularly for reference. That's why I would suggest providing an actual link to the rendered guidelines in this file, so something like that in the end:
Contributing Guidelines
=======================
For detailed contribution guidelines, please see our `Developer's Guide on ReadTheDocs <https://satpy.readthedocs.io/en/stable/dev_guide/index.html>`_.
.. If you're reading this file locally as a plain text, you may also
directly refer to the file doc/source/dev_guide/CONTRIBUTING.rst for
any unmerged/pending changes to the contribution guidelines.
Note: I also considered using the rst include statement there (ie. .. include:: doc/source/dev_guide/CONTRIBUTING.rst
) but unfortunately, this rst command is ignored by the github preview, so this would not really help in a development context [plus, it would still not use any advanced rendering trick as you mentioned above]
=> I have prepared another test branch on this, updating the two CONTRIBUTING.rst files accordingly (https://github.com/roche-emmanuel/satpy/tree/moving_contributing_rst). If this sounds OK to you then I can proceed with merging that other branch into this one (Or should I wait for the review from Martin before introducing more changes on this PR anyway ? Please let me know in this case)
@roche-emmanuel you definitely don't have to wait for me to continue :)
All modified and coverable lines are covered by tests :white_check_mark:
Project coverage is 96.05%. Comparing base (
5e27be4
) to head (9215322
). Report is 408 commits behind head on main.
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
Hi @djhoese ,
I have now merged the changes discussed above on this branch. Please let me know if you think this should be done differently when you get a chance to review this update. Thanks 😉🙏!
I believe the README link was created a long time ago when either we assumed that GitHub didn't find a README with a filename extension or it didn't actually work so we needed it. It doesn't look necessary anymore so I'm good with the removal.
This is a simple pull request to validate the changes automatically introduced with the precommit checks and fix the readthedocs automatic build accordingly.
AUTHORS.md
if not there already