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

Plugin documentation #229

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

sean-morris
Copy link
Contributor

I wrote up documentation on the plugin pieces here. It is quite detailed -- maybe too detailed. Please tell me what can be clarified. The PR is off main. I put the documentation in its own file, README-PLUGINSmd. That may not be standard practice so tell me if I should do something different. Good deal.

This addresses Issue #228

@manics
Copy link
Member

manics commented Nov 9, 2021

There's a docs folder that supports reStructuredText and Markdown/Myst:
https://github.com/jupyterhub/nbgitpuller/tree/main/docs

This is published on https://jupyterhub.github.io/nbgitpuller/

README-PLUGINS.md Outdated Show resolved Hide resolved
README-PLUGINS.md Outdated Show resolved Hide resolved
README-PLUGINS.md Outdated Show resolved Hide resolved
README-PLUGINS.md Outdated Show resolved Hide resolved
@sean-morris
Copy link
Contributor Author

Thanks for the markdown suggestions; I am getting my markdown education ramped up!

@consideRatio
Copy link
Member

Thanks for your patience with review and your work on this @sean-morris!!!

Another tip is that you can do "add suggestions to batch" and then commit multiple detail suggestions as a single commit if you inspect the changes from the "files changed" tab (and only there apparently).

Comment on lines 307 to 314
parser.add_argument('repo_dir', default='.', help='Path to clone repo under', nargs='?')
parser.add_argument('--target-dir', default='.', help='Path to clone repo under')

args = parser.parse_args()

for line in GitPuller(
args.git_url,
args.repo_dir,
branch=args.branch_name if args.branch_name else None
args.target_dir,
branch=args.branch_name
Copy link
Member

Choose a reason for hiding this comment

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

These show up again, not part of the documentation PR right?

Btw, I typically do a lot of the following to avoid accidental commits:

  • git status - what is the situation?
  • git diff - what is changed and unstaged?
  • git diff --staged - what am I about to commit?
  • git diff HEAD~1..HEAD - what did I commit in the last commit?

Then to correct mistakes I do:

  • git rebase -i HEAD~X where X is some numbers of commits to modify up in the history, and I reorder commits, change titles, stop and edits sometimes, squash/fixup etc.
  • git commit --amend or git commit --amend --no-edit to modify the latest commit etc

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, this is embarrassing! thanks, I will pull it out. In my first moments of working on this project a bunch of months ago, I worked on the command-line args piece. I made the changes off the master branch of my fork not realizing this would cause me headaches if it didn't merge in quickly.

Copy link
Member

Choose a reason for hiding this comment

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

I have a plenty of understanding with the situation @sean-morris, no worries! It is very complicated when any large or several changes are planned at the same time.

@sean-morris sean-morris force-pushed the plugin-documentation branch 2 times, most recently from 2fc249b to 312ecde Compare November 26, 2021 15:23
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.

3 participants