Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update setup docs #264

Merged
merged 9 commits into from
Aug 8, 2024
Merged

Update setup docs #264

merged 9 commits into from
Aug 8, 2024

Conversation

lochhh
Copy link
Collaborator

@lochhh lochhh commented Aug 7, 2024

Description

What is this PR

  • Bug fix
  • Addition of a new feature
  • Other

Why is this PR needed?
This PR closes #254 and #262

What does this PR do?
This PR:

  • updates the documentation to include conda installation instructions
  • replaces pip install instructions in repo README quick install with conda instructions
  • updates any mention of python 3.10 in the setup to python 3.11

References

#254 #262

How has this PR been tested?

Installed movement from conda locally and ran movement info
Docs built locally and on CI

Checklist:

  • The code has been tested locally
  • Tests have been added to cover all new functionality
  • The documentation has been updated to reflect any changes
  • The code has been formatted with pre-commit

@lochhh lochhh linked an issue Aug 7, 2024 that may be closed by this pull request
Copy link

codecov bot commented Aug 7, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 99.76%. Comparing base (8c45183) to head (99b9700).

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #264   +/-   ##
=======================================
  Coverage   99.76%   99.76%           
=======================================
  Files          14       14           
  Lines         866      866           
=======================================
  Hits          864      864           
  Misses          2        2           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@lochhh lochhh requested a review from sfmig August 7, 2024 15:47
@lochhh lochhh marked this pull request as ready for review August 7, 2024 15:47
Copy link
Contributor

@sfmig sfmig left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks @lochhh for updating! yay for spotting the inconsistencies 🦅

Mostly minor comments on phrasing, which you can ofc take or leave. I had a go at using nested tabs, to retain the previous users/developers split, and also separated install and upgrade instructions. I like it, but feel free to edit or revert, it was mostly to see how it looked.

Re movement vs movement: I will dump my current thoughts in PR #266. For this PR I think it makes sense to just make it consistent with itself. If we later decide to go monospace we will need to search / review everything anyways.

docs/source/community/roadmaps.md Show resolved Hide resolved
docs/source/getting_started/installation.md Outdated Show resolved Hide resolved
docs/source/getting_started/installation.md Outdated Show resolved Hide resolved
docs/source/getting_started/installation.md Outdated Show resolved Hide resolved
docs/source/getting_started/installation.md Outdated Show resolved Hide resolved
docs/source/getting_started/installation.md Outdated Show resolved Hide resolved
@lochhh
Copy link
Collaborator Author

lochhh commented Aug 8, 2024

Thanks @sfmig for the suggestions! I agree that the update instructions seem hidden and love the idea of splitting install and update!

Regarding the users/developers tabs, I'm thinking now, if we should just redirect developers to the contributing guide, as

  • the contributing guide has the same but more complete instructions (with the additional step to initialise the precommit hooks),
  • this increases maintainability (we don't need to update the duplicate info in two places)

If we go with the "redirect developers" method, should we have these in separate subheadings, rather than having a Developers tab which they click into, only to find that they need to click again (this is just my personal preference though 🤔). I also think it looks cleaner like this:
image

I also changed the update instructions to recommend starting fresh instead, to prevent potential incompatibility issues - happy to revert if this is unnecessary.

Lmk your thoughts! 😄

Copy link

sonarcloud bot commented Aug 8, 2024

@sfmig
Copy link
Contributor

sfmig commented Aug 8, 2024

TLDR: I agree with everything 😁

I'm thinking now, if we should just redirect developers to the contributing guide

Yes, good call! Much better for the reasons you say

If we go with the "redirect developers" method, should we have these in separate subheadings

Yes, it looks great! Feels more spacious ✨ 👩‍🚀 (and as you say less clicking, yay!)

I also changed the update instructions to recommend starting fresh instead, to prevent potential incompatibility issues - happy to revert if this is unnecessary.

Again good call, love the clarity!

Thanks @lochhh, it's looking neat! 🚀

@lochhh lochhh added this pull request to the merge queue Aug 8, 2024
Merged via the queue into main with commit e7ccfe5 Aug 8, 2024
27 checks passed
@lochhh lochhh deleted the update-setup-docs branch August 8, 2024 20:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update python version in contributing guide Document installation from conda-forge
2 participants