User notification if notebook saving fails #883

Closed
wants to merge 4 commits into
from

Conversation

Projects
None yet
4 participants
@fwerner

fwerner commented Oct 16, 2011

Hi,

a user is not notified when saving a notebook fails. I've implemented a small pop-up to fix this. Please review the changes and send me any comments.

Cheers,
Felix

@minrk

This comment has been minimized.

Show comment
Hide comment
@minrk

minrk Oct 16, 2011

Member

This is great! The notebook frontend still has yet to add error handling/reporting for most failures, but this is probably the most important.

Member

minrk commented Oct 16, 2011

This is great! The notebook frontend still has yet to add error handling/reporting for most failures, but this is probably the most important.

@fperez

This comment has been minimized.

Show comment
Hide comment
@fperez

fperez Oct 17, 2011

Member

This looks good to me, thanks a lot! @minrk, did you have anything further in mind or should we go ahead with the merge?

Member

fperez commented Oct 17, 2011

This looks good to me, thanks a lot! @minrk, did you have anything further in mind or should we go ahead with the merge?

@minrk

This comment has been minimized.

Show comment
Hide comment
@minrk

minrk Oct 17, 2011

Member

It looks good to me.

Member

minrk commented Oct 17, 2011

It looks good to me.

@fperez

This comment has been minimized.

Show comment
Hide comment
@fperez

fperez Oct 17, 2011

Member

@fwerner: could you remove that last commit ad50083? It's best not to push a merge from upsteram into the pull request, as it will make the final commit graph more complicated than necessary. You can fix the issue by doing, in your local copy of this branch:

git reset --hard c0c597c
git push --force

That will remove that final merge operation so we can bring your work into master without back-and-forth merges.

Thanks!

Member

fperez commented Oct 17, 2011

@fwerner: could you remove that last commit ad50083? It's best not to push a merge from upsteram into the pull request, as it will make the final commit graph more complicated than necessary. You can fix the issue by doing, in your local copy of this branch:

git reset --hard c0c597c
git push --force

That will remove that final merge operation so we can bring your work into master without back-and-forth merges.

Thanks!

@minrk

This comment has been minimized.

Show comment
Hide comment
@minrk

minrk Oct 17, 2011

Member

We can just merge the previous commit, there's no need for him to rollback

Member

minrk commented Oct 17, 2011

We can just merge the previous commit, there's no need for him to rollback

@fperez

This comment has been minimized.

Show comment
Hide comment
@fperez

fperez Oct 17, 2011

Member

@minrk, was it fully ready to merge? In that case I agree, no need for @fwerner to do a rollback. Just let me know and I can do it (or go ahead and do it yourself).

Member

fperez commented Oct 17, 2011

@minrk, was it fully ready to merge? In that case I agree, no need for @fwerner to do a rollback. Just let me know and I can do it (or go ahead and do it yourself).

@Carreau

This comment has been minimized.

Show comment
Hide comment
@Carreau

Carreau Mar 1, 2012

Member

The saving failed message in again pretty brief and not very visible, having the pop up back would be cool. Could Someone reopen ?

Member

Carreau commented Mar 1, 2012

The saving failed message in again pretty brief and not very visible, having the pop up back would be cool. Could Someone reopen ?

@fperez

This comment has been minimized.

Show comment
Hide comment
@fperez

fperez Mar 1, 2012

Member

Reopened.

Member

fperez commented Mar 1, 2012

Reopened.

@fperez

This comment has been minimized.

Show comment
Hide comment
@fperez

fperez Mar 1, 2012

Member

Well, for some reason I can't reopen right now. Either the wifi I'm on is really bad, or github is having a problem...

Member

fperez commented Mar 1, 2012

Well, for some reason I can't reopen right now. Either the wifi I'm on is really bad, or github is having a problem...

@minrk

This comment has been minimized.

Show comment
Hide comment
@minrk

minrk Mar 1, 2012

Member

This is a Pull Request, not a regular Issue. We don't actually want to reopen it.

Member

minrk commented Mar 1, 2012

This is a Pull Request, not a regular Issue. We don't actually want to reopen it.

@fperez

This comment has been minimized.

Show comment
Hide comment
@fperez

fperez Mar 2, 2012

Member

Ah, right.. I should stop trying to manage ipython while in a conference with 10 other things going on, sorry...

Member

fperez commented Mar 2, 2012

Ah, right.. I should stop trying to manage ipython while in a conference with 10 other things going on, sorry...

mattvonrocketstein pushed a commit to mattvonrocketstein/ipython that referenced this pull request Nov 3, 2014

Merge PR-883
Notify user if notebook save fails.

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