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

Update documentation about registering file save hooks #770

Merged
merged 1 commit into from
Apr 8, 2022

Conversation

davidbrochart
Copy link
Contributor

Follow-up to #696.

contents_manager.register_pre_save_hook(script_pre_save)
contents_manager.register_post_save_hook(script_post_save)

Hooks will then be called in the order they were registered.
Copy link
Member

Choose a reason for hiding this comment

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

Since we are now adding deprecation warnings for the old behavior, I would probably add this to the top of this document, and then put the current docs behind a "deprecated" note. Otherwise this looks good 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually only the run_pre_save_hook and run_post_save_hook methods are deprecated (which are usually not used by users), but assigning to e.g. contents_manager.pre_save_hook is not deprecated yet, so I think it is too early to discourage the old behavior in the documentation, don't you think?

@codecov-commenter
Copy link

Codecov Report

Merging #770 (92a40a4) into main (45ce912) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main     #770   +/-   ##
=======================================
  Coverage   70.36%   70.36%           
=======================================
  Files          62       62           
  Lines        7571     7571           
  Branches     1249     1249           
=======================================
  Hits         5327     5327           
  Misses       1860     1860           
  Partials      384      384           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 45ce912...92a40a4. Read the comment docs.

@Zsailer Zsailer merged commit 80ac07e into jupyter-server:main Apr 8, 2022
@davidbrochart davidbrochart deleted the doc branch April 9, 2022 07:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants