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] Refactor: Encapsulate recording path #9105

Merged
merged 3 commits into from
Feb 11, 2016

Conversation

ksooo
Copy link
Member

@ksooo ksooo commented Feb 11, 2016

No functional changes, only refactoring. Put all recordings path string manipulation magic into one specialized class.

@Jalle19 @xhaggi mind taking a look
@razzeee maybe you can supply the needed VS project files update?

@ksooo ksooo added Type: Cleanup non-breaking change which removes non-working or unmaintained functionality Component: PVR v17 Krypton labels Feb 11, 2016

bool IsValid() const { return m_bValid; }

const std::string &GetPath() const { return m_path; }

This comment was marked as spam.

This comment was marked as spam.

@ksooo ksooo force-pushed the pvr-encapsulate-recording-path branch from 37d1f39 to b8483b1 Compare February 11, 2016 08:37
@razzeee
Copy link
Member

razzeee commented Feb 11, 2016

@ksooo pick razzeee@a3fea65

@ksooo
Copy link
Member Author

ksooo commented Feb 11, 2016

thx @razzeee

@ksooo ksooo force-pushed the pvr-encapsulate-recording-path branch from b5e9000 to bedc861 Compare February 11, 2016 09:34
@xhaggi
Copy link
Member

xhaggi commented Feb 11, 2016

looks good after a quick look.

@ksooo
Copy link
Member Author

ksooo commented Feb 11, 2016

jenkins build this please

@Jalle19
Copy link
Member

Jalle19 commented Feb 11, 2016

Good improvement, thanks a lot!

@ksooo ksooo force-pushed the pvr-encapsulate-recording-path branch from bedc861 to 6a9e8ae Compare February 11, 2016 14:57
@ksooo ksooo force-pushed the pvr-encapsulate-recording-path branch from 6a9e8ae to 60c5270 Compare February 11, 2016 15:01
@ksooo
Copy link
Member Author

ksooo commented Feb 11, 2016

Rebased and CPVRRecordingsPath::Init now returning void.

@ksooo
Copy link
Member Author

ksooo commented Feb 11, 2016

jenkins build this please

ksooo added a commit that referenced this pull request Feb 11, 2016
[PVR] Refactor: Encapsulate recording path
@ksooo ksooo merged commit d940131 into xbmc:master Feb 11, 2016
@ksooo ksooo deleted the pvr-encapsulate-recording-path branch February 11, 2016 19:04
@razzeee razzeee added this to the Krypton 17.0-alpha1 milestone Feb 11, 2016
wiromare added a commit to wiromare/xbmc that referenced this pull request Feb 13, 2016
MartijnKaijser added a commit that referenced this pull request Feb 13, 2016
[win32][fix][VS] some missing end tags after #9105
@@ -1125,7 +1126,9 @@ bool CGUIWindowVideoBase::OnPlayMedia(int iItem, const std::string &player)
}
CLog::Log(LOGDEBUG, "%s %s", __FUNCTION__, CURL::GetRedacted(item.GetPath()).c_str());

if (StringUtils::StartsWith(item.GetPath(), "pvr://recordings/active/"))

CPVRRecordingsPath path(item.GetPath());

This comment was marked as spam.

This comment was marked as spam.

@ksooo
Copy link
Member Author

ksooo commented Feb 15, 2016

I agree on the second comment regarding waste if resources.

The false positives created by the change mentioned above is a stupid bug in the implementation of CPVRRecordingsPath::IsValid() which I will fix. Thanks for pointing this out.

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 v17 Krypton
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants