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

Clarify installation documentation (#1097) #1137

Merged
merged 3 commits into from Jan 9, 2021
Merged

Conversation

Seopril
Copy link
Contributor

@Seopril Seopril commented Dec 31, 2020

This is intended to fix #1097. The documentation was updated to include two notes regarding issues that could be encountered during installation. This should reduce the confusion of issues that occur when pipx is installed via apt. It also should help clarify path issues that arise when the user uses the sudo command with pipx. I've attached a screenshot of what the installation documentation looks like:

image

Checklist

  • I have read the contributing doc.
  • I have included a link to the relevant issue number.
  • I have tested this code locally.
  • I have checked to ensure there aren't other open pull requests
    for the same issue.
  • I have written new tests for these changes, as needed.
  • No test changes needed
  • All tests pass.
  • All tests passed. 5 feature scenarios were skipped and 27 feature steps were skipped when running "make test".

@@ -17,6 +17,14 @@ On other platforms, install `jrnl` using [Python](https://www.python.org/) 3.6+
pipx install jrnl
```

!!! note
`pipx` should be installed through `pip`. Missing dependencies and other issues
Copy link
Member

Choose a reason for hiding this comment

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

Can we note here that pipx's favored install methods are brew and pip (in that order)? Maybe adding another link to pipx's installation page would be a good idea (in case their installation methods change).

`pipx` should be installed through `pip`. Missing dependencies and other issues
may occur when installing `pipx` through `apt` or another package manager.

!!! note
Copy link
Member

Choose a reason for hiding this comment

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

Could this be a tip instead of a note?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just for clarification, are you referring to the first note being changed to a tip, the second note being changed to a tip, or both?

may occur when installing `pipx` through `apt` or another package manager.

!!! note
If you experience path issues after installing `jrnl`, ensure that the previous
Copy link
Member

Choose a reason for hiding this comment

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

Can we say this in a more direct way? Maybe something like "Don't use sudo to install jrnl" or something along those lines.

Copy link
Member

@wren wren left a comment

Choose a reason for hiding this comment

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

Thank you for taking this on!

I have a few notes, but it should be good to merge with minor changes.

@Seopril
Copy link
Contributor Author

Seopril commented Jan 4, 2021

No problem! I should have time to add these changes later tonight.

Copy link
Member

@wren wren left a comment

Choose a reason for hiding this comment

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

Looks good. Thank you!

@wren wren added the documentation Improvements or additions to documentation label Jan 9, 2021
@wren wren merged commit dbd12fc into jrnl-org:develop Jan 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Clarify installation docs
2 participants