Closed sfmig closed 1 month ago
All modified and coverable lines are covered by tests :white_check_mark:
Project coverage is 99.68%. Comparing base (
b2de775
) to head (3f90072
).
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
Questions/suggestions:
Should we have some automatic link checking for markdown files?
Could we include the user installation code snippet in the README? I understand we don't want to have duplicated info to avoid having to edit it in too many places if it changes, but I wonder if in this case is worth the (small?) effort. It feels slightly faffy to redirect people to the docs for just the install command (I would keep the docs link tho in case they have issues or want further details). Maybe just:
First create and activate a conda environment with the required dependencies:
conda create -n movement-env -c conda-forge python=3.10 pytables
conda activate movement-env
Then install the movement package:
pip install movement
For further details, see the installation docs.
There's already link checking right via sphinx?
yeah, but I think atm it doesn't include the repo's README. Is that right @niksirbi?
Oh yeah, sorry. That is correct (I think).
I think the two questions are related. If we keep the README minimal, as is now, I don't see much value in adding more automated checks specifically for it. If we decide to add more stuff to the README, e.g. the installation instructions that Sofia suggests, additional checks would useful.
I'm undecided on this. I agree that having the installation instructions "right there" in the README is convenient, and commonly done for other tools, but it increases the probability of things getting out of sync. There are some workarounds, like including a specific snippet of text from the README into the docs, but they are hacky and will preclude us from using the fancy sphinx-design elements.
Happy to go with whatever the majority of @neuroinformatics-unit/behaviour thinks...
My view is:
In that case, I'm happy with @sfmig adding the "Quick install" section to the README, as described above.
Sounds good! Will add it to this same PR if that's ok
Issues
0 New issues
0 Accepted issues
Measures
0 Security Hotspots
No data about Coverage
No data about Duplication
@niksirbi I had a go at removing the "table of contents" section, to have the quick install almost at the top and keep the README short - let me know what you think.
Sounds good, I'm deep in atlas-land at the moment, so I'll review this tomorrow.
Description
What is this PR
Why is this PR needed? Installation link was returning 404
What does this PR do? Fixes installation link and adds quick install instructions.
References
\
How has this PR been tested?
\
Is this a breaking change?
\
Does this PR require an update to the documentation?
\
Checklist: