Possible improvements to CONTRIBUTING and installation

[RondeauG]

• Cookiecutter version used, if any: 2023-11-28

Description

Liked to my comments in https://github.com/Ouranosinc/xscen/pull/292 and https://github.com/hydrologie/xhydro/pull/50.

• Where applicable, I'd replace conda with mamba, with maybe a brief explanation of why.
• In the Pull request instructions:
  • I'd state somewhere that once installed, pre-commit will automatically run whenever you use the git commit command.
  • Currently, the documentation makes it sound like you need to run black, tox, flake8, etc. every time.
  • Which parts of this are really necessary, since we already have pre-commit? For example with the previous version, I never actually had to run black, flake8, tox, etc. manually, since pre-commit would do it for me and prevent me from pushing my changes if it found issues with my code.
  • The only thing that was really needed was to run pytest and even then, I'd add instructions here on how to run the relevant tests only (pytest tests/test_file.py::TestClass::test_function), rather than the whole suite.
  • Those points are especially relevant when automated actions are implemented on Github, which might not always be the case. How do we manage that?
  • Currently, the documentation makes it sound like you need to run make docs every time. This is confusing for repos where we have automated actions for that. How do we also manage that?
  • I'd mention coveralls at some point and write the script for it (--cov-report html --cov .).

[Zeitsperre]

Given that we use mamba ubiquitously, I can more or less agree with that. However, this might not be the case for much longer: https://www.anaconda.com/blog/conda-is-fast-now. A counter-proposal would be to add an installation step to install conda-libmamba-solver and touch nothing else. If that ever becomes the standard, then we're back to basics.

pre-commit should be better explained, I agree (it should also be shown how to not commit with the pre-commit hooks, for certain situations). We don't typically run checks individually, so pushing pre-commit or $ make lint and $ make black are more reasonable. I can adjust that.

I can re-add the entry explaining how to run a subset of tests (I'm not sure when this was removed).

There's a way to install pre-commit hooks "globally", or rather, to make it so that if git sees that a repo has a pre-commit.config.yml, it will complain if you haven't run pre-commit install. This is beyond the scope of the project, but maybe it's worth mentioning? Beyond that, the need to run tests should be clear from the docs (no passing tests, no merge).

When I'm developing docs-related features/fixes, I typically run $ make docs every 15 minutes. The CI is absolutely necessary as a check, but I wouldn't rely on it when actively developing (needs to rebuild environment every time it is run; very slow). I still think it's a good idea to run locally.

Can add a mention for coveralls, for sure.

[RondeauG]

My issue with If you are editing the docs, compile and open them with: is that it sounds as if the only way to update the documentation is to run make docs, which simply isn't true if automatic actions have been added to the repo.

Something like this would probably be better: Upon pushing your changes to Github, the documentation will automatically be updated to reflect the changes in your pull request. However, if you are actively editing the docs, you can test your changes locally by compiling and opening them with:

However, this supposes automatic actions. It might be better to keep it as is here, but change it in xscen/xhydro.

[RondeauG]

A collaborator in xhydro (who is rather new to all of this) tried to follow the instructions, and lost multiple hours building a conda environment. So much so that he had to leave it running for the weekend!

I don't really have a strong opinion in the debate of mamba vs conda-libmamba-solver, as long as we can prevent this kind of issue!

[Zeitsperre]

Agreed. I remember you mentioning that situation. I'll test locally whether conda-libmamba-solver works for our use-cases and, if so, I'll encourage that approach.

It's a bit unintuitive to encourage users to use an alternative management tool (mamba) to normally operate conda. It isn't a one-to-one drop-in replacement for conda either, so I'd expect users who have some base conda experience would already know when to (and when not to) use it.