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

PR: Don't show internet conection errors on startup when trying to check for new releases #4237

Merged
merged 2 commits into from
Mar 10, 2017

Conversation

rlaverde
Copy link
Member

@rlaverde rlaverde commented Mar 8, 2017

Fixes #4217

@rlaverde rlaverde added this to the v3.1.4 milestone Mar 8, 2017
@rlaverde rlaverde self-assigned this Mar 8, 2017
@rlaverde rlaverde requested a review from ccordoba12 March 8, 2017 16:11
self.error = error_msg
self.sig_ready.emit()
# Don't show dialog when starting up spyder and an error occur
if not self.startup or error_msg is None:
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't this be ... or error_msg is not None?

Copy link
Member Author

Choose a reason for hiding this comment

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

Hmm no, the idea is not to show the dialog when there is an error:

Maybe it will be easy to understand:
if not (self.startup and error_message is not None):

Or
if self.startup and error_message is not None: return

Copy link
Member

Choose a reason for hiding this comment

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

Hmm no, the idea is not to show the dialog when there is an error

I don't agree. The idea (for me) is to not show the dialog if there are errors but only at startup (because it's very annoying). However, if you want to manually check for updates, Spyder should an error message in case it can't do it.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, that's the idea, the expression will evaluate True when startup is False (regardless if error_message is None or not)

Copy link
Member

Choose a reason for hiding this comment

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

Ok, but if error_msg is different from None, we also want to show the dialog. And that's not the case here because we enter this block if error_msg is None (which is a contradiction for me and what motivated me to write my first comment).

self.error = error_msg
self.sig_ready.emit()
# Don't show dialog when starting up spyder and an error occur
if not (self.startup and error_message is not None):
Copy link
Member

Choose a reason for hiding this comment

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

error_message -> error_msg

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed 😄

@rlaverde rlaverde force-pushed the update-startup-no-show-error branch from 8b6e2ca to eec7503 Compare March 10, 2017 21:15
@ccordoba12 ccordoba12 merged commit 6bb1fab into spyder-ide:3.1.x Mar 10, 2017
ccordoba12 added a commit that referenced this pull request Mar 10, 2017
ccordoba12 added a commit that referenced this pull request Mar 10, 2017
@rlaverde rlaverde deleted the update-startup-no-show-error branch March 10, 2017 22:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants