-
Notifications
You must be signed in to change notification settings - Fork 1k
docs: fix onboarding instructions and standardize troubleshooting #2851
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
docs: fix onboarding instructions and standardize troubleshooting #2851
Conversation
README.rst
Outdated
| so that others can benefit from the results. | ||
|
|
||
| If you think, that something in the code is broken/not running well, please `open an issue <https://github.com/pymodbus-dev/pymodbus/issues/new>`_, | ||
| If you think, that something in the code is broken/not running well, please open an issue, |
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.
Why are you removing the link, it is more user friendly to provide a link.
README.rst
Outdated
| If you encounter errors while running examples, please check: | ||
|
|
||
| 1. **Namespace Error** (``*** ERROR --> PyModbus not found``): | ||
| The package is not registered. Run ``pip install -e .`` from the project root. |
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.
That is not recomiendes for the dev branch, it should be as describes earlier.
README.rst
Outdated
|
|
||
| 2. **Directory Error** (``*** ERROR --> THIS EXAMPLE needs the example directory...``): | ||
| You are in the wrong folder. You **must** run the script from within the | ||
| ``examples/`` directory due to the ``helper.py`` dependency. |
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.
not only! I would remove the last sentence as it is only confusing, and especially on dev sometimes incorrect due to the use of relativo patos in between releases.
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've finalized the formatting of the README.rst by ensuring all code blocks use the correct :: syntax for proper rendering. I also removed the duplicate headers and redundant lines mentioned in the feedback.
All CI checks are green. Ready for the final review and merge! 🚀
README.rst
Outdated
| Install with pip | ||
| ^^^^^^^^^^^^^^^^ | ||
| .. note:: | ||
| This section is for library users. If you want to contribute or run |
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.
But that is wrong, you can run the examples without cloning the repo.
Please check the "Example code" chapter.
README.rst
Outdated
| ^^^^^^^^^^^^^^^^ | ||
| .. note:: | ||
| This section is for library users. If you want to contribute or run | ||
| examples from the source code, please skip to the **Install with github** section below. |
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.
Why not provide a link to the chapter, it is a lot more user friendly.
README.rst
Outdated
|
|
||
| 1. **Namespace Error** (``*** ERROR --> PyModbus not found``): | ||
| The package is not registered. Please ensure you followed the installation | ||
| steps in the **Install with github** section above. |
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.
Why not provide a link, that is more user friendly.
README.rst
Outdated
| .. note:: | ||
| This section is for library users. If you want to contribute or run | ||
| examples from the source code, please skip to the **Install with github** section below. | ||
| This section is for library users. For quick usage, check the `Example Code`_ chapter. |
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 section is "install with pip", it have nothing to do with examples !
It would be more correct to write "This section is intended for apps that uses the pymodbus library".
README.rst
Outdated
| This section is for library users. If you want to contribute or run | ||
| examples from the source code, please skip to the **Install with github** section below. | ||
| This section is for library users. For quick usage, check the `Example Code`_ chapter. | ||
| If you prefer to contribute or run advanced examples directly from the source code, |
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.
All examples, simple or advanced, can be run without cloning the repo.
README.rst
Outdated
| pip install -e ".[all]" | ||
|
|
||
| Install git hooks, that helps control the commit and avoid errors when submitting a Pull Request: | ||
| **NOTE**: The use of the ``-e`` (editable) flag is mandatory when working on the ``dev`` branch. |
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.
Why not use the markdown for note, just as you did in the "install with pip" chapter.
Also it is not "mandatory" but "recommended", the reason being that you sometimes have several branches checked out at once (worktree concept) and for that the "-e" will often point to the wrong code.
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.
Hi Jan! I've updated the PR addressing your latest feedback:
Simplified the 'Install with pip' note following your suggested wording.
Adjusted the 'Install with github' note (changed 'mandatory' to 'recommended' and used the note directive).
Fixed a duplicate header and verified the internal links.
The changes are ready for your review. Thanks for the guidance!
Description: This PR addresses inconsistencies in the README regarding the development environment setup and improves the onboarding experience for new contributors.
Key changes:
Standardized Installation: Updated the "Install with github" commands to use the editable flag (-e). This ensures that the "pointers to source" mentioned in the documentation are actually created, allowing real-time testing of core changes.
Onboarding Safety: Added a warning to the "Install with pip" section to prevent contributors from accidentally installing a static PyPI version.
Clarified Troubleshooting: Added a dedicated troubleshooting section for the dev branch. It clearly distinguishes between Namespace Errors (missing editable install) and Directory Context Errors (running examples outside the /examples folder).
These changes were verified by performing a fresh install and confirming that core modifications were correctly reflected during example execution from within the examples directory.