Threads: add missing exception handlers #921

Merged
merged 1 commit into from May 12, 2012

Conversation

Projects
None yet
3 participants
@FernetMenta
Member

FernetMenta commented May 2, 2012

No description provided.

@CrystalP

This comment has been minimized.

Show comment
Hide comment
@CrystalP

CrystalP May 3, 2012

Contributor

OK for the principle. I think there is a small problem with Win32Exception.cpp, it has a dependency on utils/log.h and it seemed that you wanted to break such dependencies with the ILogger?
There's a discussion with jcarroll about that in the IRC Log.
I'll try a few things to make win32exception use the ILogger instead.

Contributor

CrystalP commented May 3, 2012

OK for the principle. I think there is a small problem with Win32Exception.cpp, it has a dependency on utils/log.h and it seemed that you wanted to break such dependencies with the ILogger?
There's a discussion with jcarroll about that in the IRC Log.
I'll try a few things to make win32exception use the ILogger instead.

@ghost

This comment has been minimized.

Show comment
Hide comment
@ghost

ghost May 11, 2012

please pull.

ghost commented May 11, 2012

please pull.

@jimfcarroll

This comment has been minimized.

Show comment
Hide comment
@jimfcarroll

jimfcarroll May 11, 2012

Member

I think this should go. I have some work I'm adding to it that I can submit this weekend. For a preview it's here:

https://github.com/jimfcarroll/xbmc/commits/thread-cleanup

Member

jimfcarroll commented May 11, 2012

I think this should go. I have some work I'm adding to it that I can submit this weekend. For a preview it's here:

https://github.com/jimfcarroll/xbmc/commits/thread-cleanup

@jimfcarroll

This comment has been minimized.

Show comment
Hide comment
@jimfcarroll

jimfcarroll May 11, 2012

Member

Oh ... and my additions will address CrystalP's comments so we wont be adding back the circular dep

Member

jimfcarroll commented May 11, 2012

Oh ... and my additions will address CrystalP's comments so we wont be adding back the circular dep

@CrystalP

This comment has been minimized.

Show comment
Hide comment
@CrystalP

CrystalP May 11, 2012

Contributor

I didn't have time to look at the CLog -> ILogger conversion and don't mind this PR going in as is, to fix up later.
Some exception handling is better than no exception handling :)

Contributor

CrystalP commented May 11, 2012

I didn't have time to look at the CLog -> ILogger conversion and don't mind this PR going in as is, to fix up later.
Some exception handling is better than no exception handling :)

@FernetMenta

This comment has been minimized.

Show comment
Hide comment
@FernetMenta

FernetMenta May 12, 2012

Member

@jimfcarroll
Just to make sure, although I have created this pr your are the owner, right?

Member

FernetMenta commented May 12, 2012

@jimfcarroll
Just to make sure, although I have created this pr your are the owner, right?

@jimfcarroll

This comment has been minimized.

Show comment
Hide comment
@jimfcarroll

jimfcarroll May 12, 2012

Member

Ha ... I was waiting for you to merge it. I'll do it now.

Member

jimfcarroll commented May 12, 2012

Ha ... I was waiting for you to merge it. I'll do it now.

jimfcarroll pushed a commit that referenced this pull request May 12, 2012

Jim Carroll
Merge pull request #921 from FernetMenta/threads
Threads: add missing exception handlers.

I will be adding a PR to remove the circular dependency and provide a common, non-platform specific exception.

@jimfcarroll jimfcarroll merged commit 6f68bcc into xbmc:master May 12, 2012

tru added a commit to plexinc/plex-home-theater-public that referenced this pull request Dec 11, 2013

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment