-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Docs updates and improvements #6031
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
base: master
Are you sure you want to change the base?
Conversation
Poetry shell plugin, link to pull request tutorial, vim editor settings, pre-commit setup, some wording improvements.
Replaced pip with pipx (except where pip is still relevant). Re-arranged and added some clarifications about installation options, installing from sources and installing extras.
Reviewer's guide (collapsed on small PRs)Reviewer's GuideRefines contributing guidelines and modernizes installation documentation by migrating from pip to pipx, streamlining README and guides, and enhancing plugin installation instructions for extras. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey there - I've reviewed your changes - here's some feedback:
- Decide on a single authoritative link for the “great tutorial”—either use the GitHub Docs PR guide or confirm the archived link is still accurate and up-to-date.
- Verify and consolidate the pipx installation instructions across Windows, macOS, and ARM to ensure the commands work consistently on all platforms.
- Add a CHANGELOG entry summarizing the switch from pip to pipx and the other docs improvements before merging.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Decide on a single authoritative link for the “great tutorial”—either use the GitHub Docs PR guide or confirm the archived link is still accurate and up-to-date.
- Verify and consolidate the pipx installation instructions across Windows, macOS, and ARM to ensure the commands work consistently on all platforms.
- Add a CHANGELOG entry summarizing the switch from pip to pipx and the other docs improvements before merging.
## Individual Comments
### Comment 1
<location> `CONTRIBUTING.rst:345-346` </location>
<code_context>
+This will turn on syntax highlighting and load the standard vim filetype plugin
+for python files (with proper settings for indentation etc).
+
+Here's a few vim plugins that you might find useful to improve your vim python
+experience:
+
</code_context>
<issue_to_address>
**issue (typo):** Change 'Here's a few' to 'Here are a few' for subject-verb agreement.
Use 'Here are a few vim plugins' to ensure proper subject-verb agreement.
```suggestion
Here are a few vim plugins that you might find useful to improve your vim python
experience:
```
</issue_to_address>
### Comment 2
<location> `docs/guides/main.rst:68` </location>
<code_context>
-If you have pip_, just say ``pip install beets`` (or ``pip install --user
-beets`` if you run into permissions problems).
+If your distribution doesn't still ship the latest regular release of beets, you
+can install it via pipx_ in a dedicated python virtual environment, which won't
+interfere with other python packages provided by your distribution. Just
</code_context>
<issue_to_address>
**suggestion (typo):** Change 'doesn't still ship' to 'doesn't yet ship' for clarity.
'Doesn't yet ship' is more idiomatic and improves clarity.
```suggestion
If your distribution doesn't yet ship the latest regular release of beets, you
```
</issue_to_address>
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Here's a few vim plugins that you might find useful to improve your vim python | ||
experience: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
issue (typo): Change 'Here's a few' to 'Here are a few' for subject-verb agreement.
Use 'Here are a few vim plugins' to ensure proper subject-verb agreement.
Here's a few vim plugins that you might find useful to improve your vim python | |
experience: | |
Here are a few vim plugins that you might find useful to improve your vim python | |
experience: |
|
||
If you have pip_, just say ``pip install beets`` (or ``pip install --user | ||
beets`` if you run into permissions problems). | ||
If your distribution doesn't still ship the latest regular release of beets, you |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
suggestion (typo): Change 'doesn't still ship' to 'doesn't yet ship' for clarity.
'Doesn't yet ship' is more idiomatic and improves clarity.
If your distribution doesn't still ship the latest regular release of beets, you | |
If your distribution doesn't yet ship the latest regular release of beets, you |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR!
I agree that the contribution guide could use some improvements. This was also a pain point for me when I got started, it contains too much unnecessary content. In my opinion, the guide should be much smaller and more concise, with most of the details moved into the developer documentation.
It might be helpful to look at how other open source projects handle this. For example, numpyis a good reference, and I’m also pretty happy with the guides in beets-flask.
I’ve added comments with some ideas and suggestions, please don’t take them as requirements!
Regarding your help needed items:
- I would keep
guides/main.rst
as it is. There is #5807 which changes this quite a bit. plugins/index.rst
looks good to me 👍
|
||
- poetry_ for packaging, virtual environment and dependency management | ||
- poethepoet_ to run tasks, such as linting, formatting, testing | ||
- pre-commit_ to automatically run formatting tasks before each commit. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should add an optional note here as pre-commit
is not a requirement just a qol addition.
applications such as above. pipx_ installs each application in an isolated | ||
virtual environment, where its dependencies will not interfere with your system | ||
and other CLI tools. | ||
You can install these tools using your distribution's package management system, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe something like the following would be better suited. E.g. I'm using conda which works for me 🙃
For development, we recommend installing tools in isolated environments to avoid dependency conflicts. In this guide, we’ll use pipx
...
On the other hand, it may get tedious to type ``poetry run`` before every | ||
command. Instead, you can activate the virtual environment in your shell with: | ||
|
||
:: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we move all of the poetry and poe related content into a own section. Seems ill fitted in the getting the source code section.
$ poe test --lf # re-run failing tests (note the additional pytest option) | ||
$ poe check-types --pretty # check types with an extra option for mypy | ||
Finally, pre-commit_ helps us to automatically run some poe tasks before each |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This includes to much fluff in my opinion. A short mention should be enough here:
You may use
pre-commit
to automatically run all linting and formatting task before each commit. To activatepre-commit
in your local repository runpre-commit install
.
you are confused at all about how to contribute or what to contribute, take a | ||
look at `this great tutorial <http://makeapullrequest.com/>`__, or stop by our | ||
`discussion board`_ if you have any questions. | ||
look at `this great tutorial`_ or stop by our `discussion board`_ if you have |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch on the link! I normally just reference the official github guide for contribution guides.
Do you have a great bug fix, new feature, or documentation expansion you’d like | ||
to contribute? Follow these steps to create a GitHub pull request and your code | ||
will ship in no time. | ||
|
||
1. Fork the beets repository and clone it (see above) to create a workspace. | ||
2. Install pre-commit, following the instructions `here | ||
<https://pre-commit.com/>`_. | ||
3. Make your changes. | ||
4. Add tests. If you’ve fixed a bug, write a test to ensure that you’ve actually | ||
1. Fork the beets repository, clone it and install the development tools | ||
following the instructions above. | ||
2. Make your changes. | ||
3. Add tests. If you’ve fixed a bug, write a test to ensure that you’ve actually | ||
fixed it. If there’s a new feature or plugin, please contribute tests that | ||
show that your code does what it says. | ||
5. Add documentation. If you’ve added a new command flag, for example, find the | ||
4. Add documentation. If you’ve added a new command flag, for example, find the | ||
appropriate page under ``docs/`` where it needs to be listed. | ||
6. Add a changelog entry to ``docs/changelog.rst`` near the top of the document. | ||
7. Run the tests and style checker, see :ref:`testing`. | ||
8. Push to your fork and open a pull request! We’ll be in touch shortly. | ||
9. If you add commits to a pull request, please add a comment or re-request a | ||
5. Add a changelog entry to ``docs/changelog.rst`` near the top of the document. | ||
6. Run the tests and style checker, see :ref:`testing`. | ||
7. Push to your fork and open a pull request! We’ll be in touch shortly. | ||
8. If you add commits to a pull request, please add a comment or re-request a | ||
review after you push them since GitHub doesn’t automatically notify us when | ||
commits are added. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In general it feels like this could be more concise. We have already talked about many of these things already. E.g. repo should be setup...
Maybe we can reword this and put it at the top as a quick start guide for experienced open source contributors.
Editor Settings | ||
~~~~~~~~~~~~~~~ | ||
|
||
Personally, I work on beets with vim_. Here are some ``.vimrc`` lines that might | ||
help with PEP 8-compliant Python coding: | ||
Personally, I work on beets with vim_. To get a nice python working environment | ||
in vim, be sure to include the following lines in your ``.vimrc``: | ||
|
||
:: | ||
|
||
filetype indent on | ||
autocmd FileType python setlocal shiftwidth=4 tabstop=4 softtabstop=4 expandtab shiftround autoindent | ||
syntax on | ||
filetype plugin indent on | ||
|
||
Consider installing `this alternative Python indentation plugin | ||
<https://github.com/mitsuhiko/vim-python-combined>`__. I also like `neomake | ||
<https://github.com/neomake/neomake>`__ with its flake8 checker. | ||
This will turn on syntax highlighting and load the standard vim filetype plugin | ||
for python files (with proper settings for indentation etc). | ||
|
||
Here's a few vim plugins that you might find useful to improve your vim python | ||
experience: | ||
|
||
- `neomake <https://github.com/neomake/neomake>`__, with its flake8 checker. | ||
- `vim-flake8 <https://github.com/nvie/vim-flake8>`__, another interface to | ||
flake8. | ||
- `VOoM outliner <https://www.vim.org/scripts/script.php?script_id=2657>`__, can | ||
help to navigate big python files. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is my first time reading this section in detail, and I don’t think it belongs in the contributing guide. Some of the information could be moved into the developer resources in the documentation instead. In general, I think we’d be better off relocating much of this content out of the guide.
(see details here_). Beets has also been packaged in the `software | ||
repositories`_ of several distributions. Check out the `Getting Started`_ guide | ||
for more information. | ||
Beets has been packaged in the `software repositories`_ of several |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I’d keep the old README. Why remove the pip instructions? For most users, this one line and maybe a pypi link is all they need to get started. We can change it to pipx install beets
but I wouldn't remove it entirely.
Also, many distribution repositories are outdated, so I wouldn’t emphasize them, especially not in the opening sentence.
|
||
1. Uninstall beets. If you installed using ``pip``, you can just run ``pip | ||
uninstall beets``. | ||
1. Uninstall beets using the tools in your distribution, or, if you installed |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here again pip
/pipx
should take precedence over distributions.
behind, so make sure you read the right version of these docs. If you want the | ||
latest version, you can get everything you need to install with pip as | ||
described below by running: ``apt-get install python-dev python-pip`` | ||
latest version, you can get everything you need to install with pipx as |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You may want to rebase onto #5807. I improved this section slightly in the PR I'm hoping it is approved at some point 🥲
Description
Here are some documentation updates and improvements, mainly concerning beets installation and setting up a working copy for development.
f22b595 improves CONTRIBUTING.rst
poetry shell
now requires the corresponding poetry plugin to be installed57f26fe replaces pip with pipx in the documentation files
pip has long been deprecated to install end-user applications (such as beets) in favour of pipx (see PEP 668). I replaced all the occurrences where we suggest using pip to install beets, and updated the corresponding command line examples using the correct pipx options.
In some occasions, I've kept pip examples because they are still relevant (e.g. when showing how to install extras, in case a previous installation was done using pip, see below), and added the corresponding pipx examples.
help needed: BTW installation via pip is also still suggested in the beets homepage, I think we should replace it with pipx there too.
I've also made a few related improvements to some files:
README.rst
: just mention the 3 main ways to install (distro packages, pipx, github) and point toguides/main.rst
andfaq.rst
for detailed instructions (avoiding duplication)guides/main.rst
: help needed: I don't know about installation on windows, macosx and ARM; is it OK to replace pip->pipx there too? I guess so (pipx is available for windows and macosx too), but I'd like to have confirmation; please let me know and I'll update the PRplugins/index.rst
: help needed: I've also added information about how to install extras into an already existing beets installation (which I think is the most common use case: the user typically installs "plain" beets, then learns about plugins later, and then has to install extras to enable plugins). I've covered three scenarios, depending on how beets was installed (via pip, pipx, or linux distribution packages), please give some feedback about that.To Do
Tests