jupyterhub / nbgitpuller

Jupyter server extension to sync a git repository one-way to a local path
https://nbgitpuller.readthedocs.io
BSD 3-Clause "New" or "Revised" License
210 stars 86 forks source link

docs: adds docstrings (refactor: rename local parameter) #242

Closed consideRatio closed 2 years ago

consideRatio commented 2 years ago

This PR combines pure documentation commits with a refactoring commit that merits more closer review.

I'm not 100% it's non-breaking for all users to rename nbapp to app in the settings. To my knowledge, we only set it to be able to read it from this code base. For reference, setting nbapp in settings was introduced in #75.

manics commented 2 years ago

Kinda of related, there's an undocumented environment variable NBGITPULLER_APP https://github.com/jupyterhub/nbgitpuller/blob/fdd54bdd2b9c6452dcbeff8acf07a5accf7b1b64/nbgitpuller/handlers.py#L149 Do you know it's significance?

consideRatio commented 2 years ago

EDIT: Let's not discuss this here, I opened an issue #247 about it

@manics it seem to have been added by me four years ago in https://github.com/jupyterhub/nbgitpuller/pull/41 :open_mouth:

I did some git history inspection:

Overall, it seems that it does the single thing of "if set to lab", the web handler for /git-pull endpoint will default to prepending /lab/tree to a post git-pull redirection path. It won't affect gitpuller the CLI though.

I conclude that the https://jupyterhub.github.io/nbgitpuller/link only crafts links using urlpath directly though, and the NBGITPULLER_APP is a default value for a app query parameter, which only has an influence if urlpath isn't set.

I'd love to see this env var and the entire app query parameter removed to reduce complexity, but breaking existing links isn't fun. @yuvipanda do you have a suggestion with regards to something to do with the logic about having an app parameter and/or the NBGITPULLER_APP env which is the app parameters default value?

yuvipanda commented 2 years ago

Thanks a lot for this, @consideRatio!