[VideoVersion] Added VideoVersionPlay dialog

[xodidox]

Description

Added VideoVersionPlay dialog to give more flexibility to skinner
Added description for string to help translator

Motivation and context

Based on feedback

How has this been tested?

Runtime tested on MacBook

What is the effect on users?

Screenshots (if appropriate):

Types of change

• Bug fix (non-breaking change which fixes an issue)
• Clean up (non-breaking change which removes non-working, unmaintained functionality)
• Improvement (non-breaking change which improves existing functionality)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that will cause existing functionality to change)
• Cosmetic change (non-breaking change that doesn't touch code)
• Student submission (PR was done for educational purposes and will be treated as such)
• None of the above (please explain below)

Checklist:

• My code follows the Code Guidelines of this project
• My change requires a change to the documentation, either Doxygen or wiki
• I have updated the documentation accordingly
• I have read the Contributing document
• I have added tests to cover my change
• All new and existing tests passed

[ksooo]

Hi, @xodidox

Added description for string to help translator

That's not going to work like you want. Weblate does not support a group comment for multiple strings (As far as I know). Translaters never look at the strings.po directly, they are using Weblate. The string you've added will be displayed in Weblate along with string #40400. You need to add a descriptive text (.# ) for every single string.

Added VideoVersionPlay dialog to give more flexibility to skinner

This dialog seems to be a dummy, without any functionality. What exactly can skinners make out of that? My skinning skills are really limited, so I'm just asking.

EDIT: nvm, overlooked that this is not a new dialog class.

[ksooo]

This is not going to work. See my other comment.

[xodidox]

I'll update this. The discussion for VideoVersionPlay
is from https://forum.kodi.tv/showthread.php?tid=337992&pid=3172621#pid3172621

[da-anda]

in order for your PRs to be easer to review it would be good if you could split up the changes into logical commits. Like in this PR one commit for the dialog and a second commit for the label descriptions. Since this PR is rather small, it might be a good exercise on the workflow for this, in case you are not that familiar with GIT yet.

[ksooo]

Yes, that would indeed be helpful, to back what @da-anda said.

[jjd-uk]

I've done a functional test, and it does exactly what me and @HitcherUK asked for. Thanks @xodidox

[Hitcher]

Tested videoversion and videoversionplay windows and everything works as excepted.

I guess you can drop the label (id1) now as skins can use Window.IsActive(videoversion) or Window.IsActive(videoversionplay) for conditions.

Thanks.

[xodidox]

in order for your PRs to be easer to review it would be good if you could split up the changes into logical commits. Like in this PR one commit for the dialog and a second commit for the label descriptions. Since this PR is rather small, it might be a good exercise on the workflow for this, in case you are not that familiar with GIT yet.

Thanks for this tip. I'm not sure when and how, but I got the impression that each PR should only have one commit for Kodi repo. Maybe I was asked to squash commits? Don't remember. I'll adopt this in future PRs.

[jjd-uk]

For review it's easier for the changes to be in logical chunks. Once review is complete and PR is ready to be merged then it can be squashed down to one commit.

[ksooo]

The point is one commit per logical change , not one commit per PR. A PR can contain multiple logical changes, for example fix A and cleanup B. We want to have separate commits for those changes, one for the fix one for the cleanup.

[jurialmunkey]

Thanks for this tip. I'm not sure when and how, but I got the impression that each PR should only have one commit for Kodi repo. Maybe I was asked to squash commits? Don't remember. I'll adopt this in future PRs.

Possibly the confusion stems from the plugins and scripts addon repos having this requirement to squash first. If you've ever submitted an addon update then you would've been asked to squash
https://github.com/xbmc/repo-plugins/blob/master/CONTRIBUTING.md#general-guidelines-for-creating-pull-requests
https://github.com/xbmc/repo-scripts/blob/master/CONTRIBUTING.md#general-guidelines-for-creating-pull-requests

I have a feeling this is a relic from before github introduced the "squash and merge" button.