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

WIP Quick doc #1608

Merged
merged 38 commits into from Jul 20, 2023
Merged

WIP Quick doc #1608

merged 38 commits into from Jul 20, 2023

Conversation

LeoDaCoda
Copy link
Contributor

@LeoDaCoda LeoDaCoda commented Jul 10, 2023

Hey, this PR is in reference to an issue I raised. I would like to add a quick start guide to act as another resource for developers, I planned the guide to be mainly code snippets as opposed to a text based tutorial.

I finished all the code and section headers I planned to add to the quick doc. I still want to add a few short sentences to improve readability. The tutorials includes how to do basic repo operations like init, clone, add, commit and push. Also shows how to iterate through the trees and print a text file.

Where possible I tried to follow the conventions the regular tutorial followed (ie having the code snippets inside of the test file) but in some cases I had to add code directly to the RST (I did this to avoid having unnecessary print statements to the test suite).

This is my first PR so forgive me if I did not follow propper conventions. Look forward to your feedback, Thanks!

Copy link
Member

@Byron Byron left a comment

Choose a reason for hiding this comment

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

Thanks a lot for taking the time to figure out how to contribute to the documentation setup used in GitPython, it's absolutely awesome to see it improve after all this time.

The goal of the quick-start section is to show code people can copy-paste and as such it should represent the best GitPython, and python, have to offer. Thus I may have a lot of opinions in the following comments, but those are merely to get closer to that (probably unattainable) goal.

Generally, please avoid assigning variables that are only used once unless they truly help to explain what the expression means, and names like fileX don't seem to do that.

Besides that, I think this is an excellent basis for getting this out to people fast. What would help me is if you could attach a PDF of the rendered document in the comments so I don't have to go through the trouble of building it locally (I have not been able to do anything with python for quite some time and it's great pain for me to deal with that again).

Looking forward to push this PR to an early completion, hoping that you can keep improving the document as you gain more insights into GitPython yourself.

doc/source/quickstart.rst Show resolved Hide resolved
doc/source/quickstart.rst Outdated Show resolved Hide resolved
doc/source/quickstart.rst Outdated Show resolved Hide resolved
doc/source/quickstart.rst Outdated Show resolved Hide resolved
test/test_quick_doc.py Outdated Show resolved Hide resolved
test/test_quick_doc.py Show resolved Hide resolved

# Previous commit tree
# [13-test_cloned_repo_object]
prev_commits = [c for c in repo.iter_commits('--all', max_count=10)]
Copy link
Member

Choose a reason for hiding this comment

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

Isn't there a way to get the last result of an iterator without creating an intermediate array? If not, and it looks like it, please ignore and leave it.

test/test_quick_doc.py Outdated Show resolved Hide resolved
test/test_quick_doc.py Outdated Show resolved Hide resolved
test/test_quick_doc.py Outdated Show resolved Hide resolved
@LeoDaCoda
Copy link
Contributor Author

Sorry for the late response I was a little busy this week. I think I addressed all your comments and I attached the pdf output. I asked gpt to write a 1-2 sentence intro, but to me it still sounds a little wonky.

I intended the quickstart`s audience to be developers that want to just code/experiment and do not want to comb through the entire tutorial and stackoverflow posts. Do you think the tutorial is missing anything
Quickstar-print.pdf

Copy link
Member

@Byron Byron left a comment

Choose a reason for hiding this comment

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

Thanks a lot for making time and improving it to the point of "nearly there". The PDF was key to a much more pleasant review, thanks for that.

I have a left a couple of comments, but would additionally like you to check each sentence and make sure there is a period (.) terminating them. The remaining bullet-point-style sentences should also be changed to full sentences, for consistency. Now that I read through it, maybe it's easier to turn these sentences into bulletpoints, please make your choice.

Thanks again for your help.

doc/source/quickstart.rst Outdated Show resolved Hide resolved
test/test_quick_doc.py Show resolved Hide resolved
doc/source/quickstart.rst Outdated Show resolved Hide resolved
test/test_quick_doc.py Show resolved Hide resolved
commits_for_file

# Outputs: [<git.Commit "SHA1-HEX-HASH-1">,
# <git.Commit "SHA1-HEX-HASH-1">]
Copy link
Member

Choose a reason for hiding this comment

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

That would be "SHA1-HEX_HASH-2".

test/test_quick_doc.py Outdated Show resolved Hide resolved
doc/source/quickstart.rst Show resolved Hide resolved
test/test_quick_doc.py Outdated Show resolved Hide resolved
@LeoDaCoda
Copy link
Contributor Author

Hey Byron! I made some changes to make the document more readable like putting the equivalent bash in a comment instead as a sub-header. I also some made general changes to the structure and the order of the code-snippets.

I attached the pdf print version.
Quickstar-print.pdf

Copy link
Member

@Byron Byron left a comment

Choose a reason for hiding this comment

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

Thanks again! I have a feeling that not all of my comments have been applied (I am looking at you, . ;)), but with the structural changes it doesn't strike me as problem anymore.

Once you have applied the last remaining changes to unify the depiction of hashes, this PR is good to go. Note that the idea there was to not let it appear they are the same by adding an incremental number at their tail, so now I go for the smallest common denominator and remove the numbering instead so they are at least consistent.

test/test_quick_doc.py Show resolved Hide resolved
test/test_quick_doc.py Outdated Show resolved Hide resolved
test/test_quick_doc.py Outdated Show resolved Hide resolved
@LeoDaCoda
Copy link
Contributor Author

LeoDaCoda commented Jul 19, 2023

At first I didn't understand what you meant by the -1 and -2 but after you explained it it made more sense. I tried to follow this convention: blob and tree would not have an appended number, but commits would be numbered in chronological order.

I also wanted to link some sections of the full tutorial in the relevant sections of the quick start, but I ran into a problem. Sphinx requires references to follow this convention: :ref:'Remotes <handling-remotes-label>' where the label .. _handling-remotes-label: exists in some other RST document. Do you think its a good idea to add labels to the subsections of the full tutorial?
Quickstar-print.pdf

@Byron Byron added this to the v3.1.33 - Bugfixes milestone Jul 20, 2023
@Byron
Copy link
Member

Byron commented Jul 20, 2023

Great, thanks for all the fixes!

Regarding the subsection labels, feel free to add them in the next PR :). Let's get this merged!

@Byron Byron merged commit 09861ea into gitpython-developers:main Jul 20, 2023
7 checks passed
renovate bot added a commit to allenporter/flux-local that referenced this pull request Sep 4, 2023
[![Mend
Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Change | Age | Adoption | Passing | Confidence |
|---|---|---|---|---|---|
| [GitPython](https://togithub.com/gitpython-developers/GitPython) |
`==3.1.32` -> `==3.1.34` |
[![age](https://developer.mend.io/api/mc/badges/age/pypi/GitPython/3.1.34?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![adoption](https://developer.mend.io/api/mc/badges/adoption/pypi/GitPython/3.1.34?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![passing](https://developer.mend.io/api/mc/badges/compatibility/pypi/GitPython/3.1.32/3.1.34?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![confidence](https://developer.mend.io/api/mc/badges/confidence/pypi/GitPython/3.1.32/3.1.34?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|

---

### Release Notes

<details>
<summary>gitpython-developers/GitPython (GitPython)</summary>

###
[`v3.1.34`](https://togithub.com/gitpython-developers/GitPython/releases/tag/3.1.34):
- fix resource leaking

[Compare
Source](https://togithub.com/gitpython-developers/GitPython/compare/3.1.33...3.1.34)

##### What's Changed

- util: close lockfile after opening successfully by
[@&#8203;skshetry](https://togithub.com/skshetry) in
[gitpython-developers/GitPython#1639

##### New Contributors

- [@&#8203;skshetry](https://togithub.com/skshetry) made their first
contribution in
[gitpython-developers/GitPython#1639

**Full Changelog**:
gitpython-developers/GitPython@3.1.33...3.1.34

###
[`v3.1.33`](https://togithub.com/gitpython-developers/GitPython/releases/tag/3.1.33):
- with security fix

[Compare
Source](https://togithub.com/gitpython-developers/GitPython/compare/3.1.32...3.1.33)

##### What's Changed

- WIP Quick doc by [@&#8203;LeoDaCoda](https://togithub.com/LeoDaCoda)
in
[gitpython-developers/GitPython#1608
- Partial clean up wrt mypy and black by
[@&#8203;bodograumann](https://togithub.com/bodograumann) in
[gitpython-developers/GitPython#1617
- Disable merge_includes in config writers by
[@&#8203;bodograumann](https://togithub.com/bodograumann) in
[gitpython-developers/GitPython#1618
- feat: full typing for "progress" parameter in Repo class by
[@&#8203;madebylydia](https://togithub.com/madebylydia) in
[gitpython-developers/GitPython#1634
- Fix CVE-2023-40590 by
[@&#8203;EliahKagan](https://togithub.com/EliahKagan) in
[gitpython-developers/GitPython#1636
-
[#&#8203;1566](https://togithub.com/gitpython-developers/GitPython/issues/1566)
Creating a lock now uses python built-in "open()" method to work arou…
by [@&#8203;HageMaster3108](https://togithub.com/HageMaster3108) in
[gitpython-developers/GitPython#1619

##### New Contributors

- [@&#8203;LeoDaCoda](https://togithub.com/LeoDaCoda) made their first
contribution in
[gitpython-developers/GitPython#1608
- [@&#8203;bodograumann](https://togithub.com/bodograumann) made their
first contribution in
[gitpython-developers/GitPython#1617
- [@&#8203;EliahKagan](https://togithub.com/EliahKagan) made their first
contribution in
[gitpython-developers/GitPython#1636
- [@&#8203;HageMaster3108](https://togithub.com/HageMaster3108) made
their first contribution in
[gitpython-developers/GitPython#1619

**Full Changelog**:
gitpython-developers/GitPython@3.1.32...3.1.33

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined),
Automerge - At any time (no schedule defined).

🚦 **Automerge**: Enabled.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the
rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update
again.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR has been generated by [Mend
Renovate](https://www.mend.io/free-developer-tools/renovate/). View
repository job log
[here](https://developer.mend.io/github/allenporter/flux-local).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNi43OC44IiwidXBkYXRlZEluVmVyIjoiMzYuNzguOCIsInRhcmdldEJyYW5jaCI6Im1haW4ifQ==-->

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
otc-zuul bot pushed a commit to opentelekomcloud-infra/grafana-docs-monitoring that referenced this pull request Sep 11, 2023
Bump gitpython from 3.1.32 to 3.1.35

Bumps gitpython from 3.1.32 to 3.1.35.

Release notes
Sourced from gitpython's releases.

3.1.35 - a fix for CVE-2023-41040
What's Changed

Bump actions/checkout from 3 to 4 by @​dependabot in gitpython-developers/GitPython#1643
Fix 'Tree' object has no attribute '_name' when submodule path is normal path by @​CosmosAtlas in gitpython-developers/GitPython#1645
Fix CVE-2023-41040 by @​facutuesca in gitpython-developers/GitPython#1644
Only make config more permissive in tests that need it by @​EliahKagan in gitpython-developers/GitPython#1648
Added test for PR #1645 submodule path by @​CosmosAtlas in gitpython-developers/GitPython#1647
Fix Windows environment variable upcasing bug by @​EliahKagan in gitpython-developers/GitPython#1650

New Contributors

@​CosmosAtlas made their first contribution in gitpython-developers/GitPython#1645
@​facutuesca made their first contribution in gitpython-developers/GitPython#1644

Full Changelog: gitpython-developers/GitPython@3.1.34...3.1.35
3.1.34 - fix resource leaking
What's Changed

util: close lockfile after opening successfully by @​skshetry in gitpython-developers/GitPython#1639

New Contributors

@​skshetry made their first contribution in gitpython-developers/GitPython#1639

Full Changelog: gitpython-developers/GitPython@3.1.33...3.1.34
v3.1.33 - with security fix
What's Changed

WIP Quick doc by @​LeoDaCoda in gitpython-developers/GitPython#1608
Partial clean up wrt mypy and black by @​bodograumann in gitpython-developers/GitPython#1617
Disable merge_includes in config writers by @​bodograumann in gitpython-developers/GitPython#1618
feat: full typing for "progress" parameter in Repo class by @​madebylydia in gitpython-developers/GitPython#1634
Fix CVE-2023-40590 by @​EliahKagan in gitpython-developers/GitPython#1636
#1566 Creating a lock now uses python built-in "open()" method to work arou… by @​HageMaster3108 in gitpython-developers/GitPython#1619

New Contributors

@​LeoDaCoda made their first contribution in gitpython-developers/GitPython#1608
@​bodograumann made their first contribution in gitpython-developers/GitPython#1617
@​EliahKagan made their first contribution in gitpython-developers/GitPython#1636
@​HageMaster3108 made their first contribution in gitpython-developers/GitPython#1619

Full Changelog: gitpython-developers/GitPython@3.1.32...3.1.33



Commits

c8e303f prepare next release
09e1b3d Merge pull request #1650 from EliahKagan/envcase
8017421 Merge pull request #1647 from CosmosAtlas/master
fafb4f6 updated docs to better describe testing procedure with new repo
9da24d4 add test for submodule path not owned by submodule case
eebdb25 Eliminate duplication of git.util.cwd logic
c7fad20 Fix Windows env var upcasing regression
7296e5c Make test helper script a file, for readability
d88372a Add test for Windows env var upcasing regression
11839ab Merge pull request #1648 from EliahKagan/file-protocol
Additional commits viewable in compare view




Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting @dependabot rebase.


Dependabot commands and options

You can trigger Dependabot actions by commenting on this PR:

@dependabot rebase will rebase this PR
@dependabot recreate will recreate this PR, overwriting any edits that have been made to it
@dependabot merge will merge this PR after your CI passes on it
@dependabot squash and merge will squash and merge this PR after your CI passes on it
@dependabot cancel merge will cancel a previously requested merge and block automerging
@dependabot reopen will reopen this PR if it is closed
@dependabot close will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually
@dependabot show <dependency name> ignore conditions will show all of the ignore conditions of the specified dependency
@dependabot ignore this major version will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself)
@dependabot ignore this minor version will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself)
@dependabot ignore this dependency will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself)
You can disable automated security fix PRs for this repo from the Security Alerts page.

Reviewed-by: Vladimir Vshivkov
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants