Correct debug info + spelling/grammar/style #82
Conversation
There was a problem hiding this comment.
Sorry it took me time to come back to you. I've been quite busy recently :)
I added a few comments inline for this patch, and a global comment for the other PR. Don't worry for the failing tests as they are clearly not related to README changes. I can merge even without them passing there.
README.md
Outdated
| @@ -66,23 +66,23 @@ GNU General Public License for more details](https://www.gnu.org/licenses/gpl-2. | |||
| As I'm a fervent [Linux](https://www.kernel.org/) user, this version of the script has only been used and tested on Linux [Debian](https://www.debian.org/). I will not be able to maintain directly the [Windows](http://windows.microsoft.com) compatibility but I'm open to pull requests on this side, as we're here to share. | |||
|
|
|||
| #### Python version | |||
| This version of TraktForVLC has been tested using Python 2.7.9 and Python 3.4.2 It should however be compatible with the following versions: | |||
| This version of TraktForVLC has been tested using Python 2.7.9 and Python 3.4.2. It should, however, be compatible with the following versions: | |||
| + Python 2.6 | |||
There was a problem hiding this comment.
As you're in this file... can you please update the list of Python versions ? :)
- Python 2.7
- Python 3.3
- Python 3.4
- Python 3.5
- Python 3.6
- Python 3.7
(Thus, not 2.6 anymore, it was removed from the tests)
README.md
Outdated
| + Python 2.6 | ||
| + Python 2.7 | ||
| + Python 3.3 | ||
| + Python 3.4 | ||
|
|
||
| #### VLC version | ||
| This version of TraktForVLC has been tested using VLC 2.2.0 Weatherwax | ||
| This version of TraktForVLC has been tested using VLC 2.2.0 Weatherwax. |
There was a problem hiding this comment.
The VLC version can be updated too: VLC 2.2.6 Umbrella
README.md
Outdated
| @@ -246,6 +246,6 @@ and will automatically be started for subsequent system boots. | |||
| ## Issues | |||
| Please use the [GitHub integrated issue tracker](https://github.com/XaF/TraktForVLC/issues) for every problem you can encounter. Please **DO NOT** use my email for issues or walkthrough. | |||
|
|
|||
| When submitting an issue, please submit a TraktForVLC logfile showing the error. You can start TraktForVLC in debug mode (```--debug``` option) to obtain more thorough logs and with small timers (```--small-timers``` option) to speed up its internal loop. The resulting logfile (```TraktForVLC-DEBUG.log```) should then be available in your ```logs/``` directory. | |||
| When submitting an issue, please submit a TraktForVLC logfile showing the errors you are running into. You can run TraktForVLC in debug mode (```--debug``` option) for more thorough logs when debugging manually. To save these contents into a separate logfile (```TraktForVLC-DEBUG.log```) in your ```logs/``` directory, use the ```--daemon``` option or ```--pidfile some_file.txt``` when running the script. You can additionally use the option ```--small-timers``` to speed up the script's internal loop. | |||
There was a problem hiding this comment.
Can you merge these changes with the other commit ?
There was a problem hiding this comment.
Which one? (Sorry, this is showing as outdated, so didn't see the comment right away and I'm confused now, ha.)
|
Hi, because of the changed line numbers (with vs. without wrap) it's impossible to undo the removal of the line breaks and apply any of the content changes in this or the other PR without lots of extra work, so it will be easier if I reintroduce line breaks. Please tell me at which character I should break. I have my rulers set to 78 for Python code and the max. recommendation (for console work exactly) is 80 iirc, but the current README on master goes quite a bit over that limit, and very inconsistently so, with some lines having > 90 characters even. |
|
It's highly probable some of my changes in the README did not keep in sync with what should have been done, but I usually break at 80 (i.e. no line with 81 characters or more). Also, please rebase your branch and squash all your commits in one (currently there is 4 of those) |
|
Added the line breaks back in. The branch was/is up-to-date with master, will squash the commits once I got your final OK with the current changes. (As far as I know GH PRs can also be squashed into one commit when accepting them though I don't have experience with that.) |
|
(In case this is something you wanna enable: Configuring commit squashing for pull requests) |
|
I usually don't merge directly from GitHub. I don't know how the "squash and merge" process works and what will be the resulting commit message, so I prefer to avoid it. I usually merge by hand, so if you may squash them, I'll merge them after! |
|
Oh, interesting, I hadn't even considered merges not done on GH, so yeah, makes sense. In any case, I've now squashed them all down into one commit! |
|
I just had a look at the AppVeyor logs, looks like the build failing didn't have anything to do with the code? I haven't checked all of the Travis build instances, but what I saw was pointing to a problem with the test file... |
|
As I said in one of the issues, because of everything else failing in TraktForVLC 1.x, I chose the path of giving more time on TraktForVLC 2.x to have a working TraktForVLC as fast as possible. I did not include your changes in the master as I did not take the time to fix the other problems lying around in 1.x Sorry for that! TraktForVLC 2.x makes that PR obsolete - #88 |
builds on #81; rewords confusing/misleading debug/log info at end of README + adds minor other fixes (spelling etc.)