search

XaF / TraktForVLC

Automatically trakt.tv what you're watching on VLC
300 stars 49 forks source link

Correct debug info + spelling/grammar/style #82

Closed keikoro closed 6 years ago

keikoro commented 6 years ago

builds on #81; rewords confusing/misleading debug/log info at end of README + adds minor other fixes (spelling etc.)

keikoro commented 6 years ago

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.

XaF commented 6 years ago

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)

keikoro commented 6 years ago

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.)

keikoro commented 6 years ago

(In case this is something you wanna enable: Configuring commit squashing for pull requests)

XaF commented 6 years ago

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!

keikoro commented 6 years ago

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!

keikoro commented 6 years ago

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...

XaF commented 6 years ago

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