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

minor checkpoint cleanup #3939

Merged
merged 1 commit into from Aug 10, 2013
Merged

minor checkpoint cleanup #3939

merged 1 commit into from Aug 10, 2013

Conversation

minrk
Copy link
Member

@minrk minrk commented Aug 7, 2013

  • remember list of checkpoints browser-side
  • don't clobber list when a new checkpoint is created
  • cleanup references in MenuBar restore list.
    There was a closure issue, where multiple menu items would actually all
    restore the same checkpoint.

Issues revealed by rgbkrk/bookstore, which supports multiple checkpoints.

I'm fine if this doesn't get into 1.0, given timing.

- remember list of checkpoints browser-side
- cleanup references in MenuBar restore list.
  There was a closure issue, where multiple menu items would actually all restore the same checkpoint.
@minrk
Copy link
Member Author

minrk commented Aug 7, 2013

@rgbkrk, if you want to test this with your repo, I think it should be better behaved.

@rgbkrk
Copy link
Member

rgbkrk commented Aug 7, 2013

Neat! I'll test against this commit.

@rgbkrk
Copy link
Member

rgbkrk commented Aug 7, 2013

This is awesome! It's totally seamless and works flawlessly. Was able to restore to several different checkpoints while working with a notebook. It all worked as it should.

@rgbkrk
Copy link
Member

rgbkrk commented Aug 8, 2013

👍 Is there some amount of testing or other work I could do to get this PR into 1.0?

@minrk
Copy link
Member Author

minrk commented Aug 8, 2013

I think this has missed the cutoff for 1.0. I will make sure it's in the first 1.x bugfix release, though.

@rgbkrk
Copy link
Member

rgbkrk commented Aug 8, 2013

Cool. I'll just have bookstore release with single checkpoint for now (multi is on GitHub, not on PyPI yet).

@Carreau
Copy link
Member

Carreau commented Aug 10, 2013

Looks good to me, and seeing comment, merging.

Carreau added a commit that referenced this pull request Aug 10, 2013
minor checkpoint cleanup

remember list of checkpoints browser-side
don't clobber list when a new checkpoint is created
cleanup references in MenuBar restore list. There was a closure issue, where multiple menu items would actually all restore the same checkpoint.
@Carreau Carreau merged commit dbf7918 into ipython:master Aug 10, 2013
minrk added a commit that referenced this pull request Aug 10, 2013
- remember list of checkpoints browser-side
- don't clobber list when a new checkpoint is created
- cleanup references in MenuBar restore list.
  There was a closure issue, where multiple menu items would actually all
  restore the same checkpoint.

Issues revealed by rgbkrk/bookstore, which supports multiple checkpoints.

I'm fine if this doesn't get into 1.0, given timing.
mattvonrocketstein pushed a commit to mattvonrocketstein/ipython that referenced this pull request Nov 3, 2014
minor checkpoint cleanup

remember list of checkpoints browser-side
don't clobber list when a new checkpoint is created
cleanup references in MenuBar restore list. There was a closure issue, where multiple menu items would actually all restore the same checkpoint.
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.

None yet

3 participants