Skip to content
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

[PVR] PVRManager: Factor out playback state functionality into its own class #16699

Merged
merged 1 commit into from Oct 2, 2019

Conversation

ksooo
Copy link
Member

@ksooo ksooo commented Oct 2, 2019

Another PVR refactoring PR. Idea is decoupling and separation of concerns.

In the distant future PVR shall no more know anything about any application playback states. So, this PR is a step into this direction.

Runtime-tested on macOS and Android, latest Kodi master.

@ksooo ksooo added Type: Cleanup non-breaking change which removes non-working or unmaintained functionality Component: PVR v19 Matrix labels Oct 2, 2019
@ksooo ksooo added this to the Matrix 19.0-alpha 1 milestone Oct 2, 2019
@ksooo ksooo requested a review from phunkyfish October 2, 2019 12:45
Rechi
Rechi previously requested changes Oct 2, 2019
Copy link
Member

@Rechi Rechi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you prefer the style you used in this PR, crate a PR with documentation and clang-format configuration changes.

xbmc/pvr/PVRPlaybackState.h Outdated Show resolved Hide resolved
xbmc/pvr/PVRPlaybackState.cpp Outdated Show resolved Hide resolved
Copy link
Contributor

@phunkyfish phunkyfish left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Outside of the left align from Rechi it looks good. Not sure if my comments originate from NULL or nullptr so maybe the comment not so important. Up to you.

xbmc/pvr/PVRPlaybackState.h Outdated Show resolved Hide resolved
xbmc/pvr/PVRPlaybackState.h Outdated Show resolved Hide resolved
xbmc/pvr/PVRPlaybackState.h Outdated Show resolved Hide resolved
@ksooo
Copy link
Member Author

ksooo commented Oct 2, 2019

If you prefer the style you used in this PR, crate a PR with documentation and clang-format configuration changes.

For the records: I do not accept any clang-format stuff as a blocker for any PR. Team Kodi developers have no common agreement or at least consensus on the rule set you're trying to enforce here by blocking a PR.

I will incorporate all requested changes except the ctor initlist reformat stuff and merge as soon as Jenkins is happy.

@Rechi
Copy link
Member

Rechi commented Oct 2, 2019

Team Kodi developers have no common agreement or at least consensus on the rule set

That's the reason I'm asking you to submit a PR with your suggestions. Otherwise everyone wants to keep his preferred code style.

@ksooo
Copy link
Member Author

ksooo commented Oct 2, 2019

Sigh. Not worth the fight. I have better things to do. I will just obey the existing rules.

@ksooo ksooo force-pushed the pvr-refactor-playbackstate branch from 992bf7b to 321f2e3 Compare October 2, 2019 14:26
@ksooo
Copy link
Member Author

ksooo commented Oct 2, 2019

Here we go. All (!) requested changes are in. Please approve or comment on other findings.

@Rechi Rechi dismissed their stale review October 2, 2019 14:30

code follows now current coding guidelines and clang-format configuration

@ksooo ksooo merged commit 35b2594 into xbmc:master Oct 2, 2019
@ksooo ksooo deleted the pvr-refactor-playbackstate branch October 2, 2019 14:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: PVR Type: Cleanup non-breaking change which removes non-working or unmaintained functionality v19 Matrix
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants