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

Remove value constructors for ConsoleValueRef & fix callbacks #1298

Closed
wants to merge 1 commit into
base: development
from

Conversation

Projects
None yet
2 participants
@jamesu
Contributor

jamesu commented May 6, 2015

Fixes issue #1289

@@ -129,7 +129,7 @@ ConsoleValueRef SimConsoleThreadExecCallback::waitForResult()
return retVal;
}
return (const char*)NULL;
return ConsoleValueRef();

This comment has been minimized.

@crabmusket

crabmusket May 7, 2015

Contributor

As far as I can tell, this should never be called, right?

@crabmusket

crabmusket May 7, 2015

Contributor

As far as I can tell, this should never be called, right?

This comment has been minimized.

@jamesu

jamesu May 7, 2015

Contributor

I'm guessing no but it doesn't make sense to return retVal if for whatever reason acquire fails.

@jamesu

jamesu May 7, 2015

Contributor

I'm guessing no but it doesn't make sense to return retVal if for whatever reason acquire fails.

This comment has been minimized.

@crabmusket

crabmusket May 7, 2015

Contributor

Yep, I was just trying to understand the flow.

@crabmusket

crabmusket May 7, 2015

Contributor

Yep, I was just trying to understand the flow.

@crabmusket crabmusket added the Bug label May 7, 2015

@crabmusket crabmusket added this to the 3.7 milestone May 7, 2015

@crabmusket

This comment has been minimized.

Show comment
Hide comment
@crabmusket

crabmusket May 7, 2015

Contributor

Will merge this manually after review.

Contributor

crabmusket commented May 7, 2015

Will merge this manually after review.

@crabmusket

This comment has been minimized.

Show comment
Hide comment
@crabmusket

crabmusket May 7, 2015

Contributor

Yay it works. Thanks @jamesu! Merged in 12d435c.

Contributor

crabmusket commented May 7, 2015

Yay it works. Thanks @jamesu! Merged in 12d435c.

@crabmusket crabmusket closed this May 7, 2015

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