Added additional metadata to diff output by banginji · Pull Request #46 · target/diff-poetry-lock

banginji · 2026-02-26T05:18:37Z

Added in current and new commit hashes
Added the current version of the tool

This fixes #31

Added in current and new commit hashes Added the current version of the tool Signed-off-by: banginji <ban.ginji@gmail.com>

Signed-off-by: banginji <ban.ginji@gmail.com>

colindean · 2026-03-02T18:36:48Z

diff_poetry_lock/run_poetry.py

suggestion: I think I'd prefer this ordering and format for the new stuff, plus some minor cleanup:

Suggested change

change_count = len(added + removed + updated)

comment = f"### Detected {change_count} changes to dependencies in Poetry lockfile\n\n"

if base_commit_hash and target_commit_hash:

# let GitHub auto-link the hashes

comment += f"From base {base_commit_hash} to target {target_commit_hash}:\n\n"

summary_lines = [p.summary_line() for p in added + removed + updated]

comment += "\n".join(summary_lines)

comment += (

f"\n\n*({len(added)} added, {len(removed)} removed, {len(updated)} updated, {len(not_changed)} not changed)*"

)

if diff_poetry_lock_version:

comment += f"<small>Generated by diff-poetry-lock {diff_poetry_lock_version}</small>\n\n"

That check and use of diff_poetry_lock_version is actually redundant: Just use __version__ directly since it's in scope. I don't really see a testing reason to override it.

Updated to follow suggested format for diff output and replaced version with __version__

colindean · 2026-03-02T18:40:07Z

diff_poetry_lock/run_poetry.py

+        base_commit_hash = api.resolve_commit_hash(settings.ref)
+        target_commit_hash = api.resolve_commit_hash(settings.base_ref)


question: Hmm. This adds two HTTP requests to this process, right? Can we resolve the hashes without touching the API, or ensure that the result is served from some cached value since api.resolve_commit_hash() could hit a cache that may be populated in inside of load_lockfile()?

As in, when we load the lockfile given a ref, can we get the hash of the ref as a side benefit, then put that into a cache?

Updated to make get_file to now return a json that has the sha# which is persisted in the github class and is looked up here so removed the 2 additional apis and the newly implemented api call

colindean · 2026-03-02T18:42:17Z

diff_poetry_lock/run_poetry.py

+            packages,
+            base_commit_hash=base_commit_hash,
+            target_commit_hash=target_commit_hash,
+            diff_poetry_lock_version=__version__,


thought: This is about a parameter away from benefitting from some kind of a CommentData class. But, I think you can drop the diff_poetry_lock_version=__version__ since __version__ is in scope… or you can make it a default value for the parameter and avoid passing it here.

Removed the parameter; __version__ is inlined in the method now

colindean · 2026-03-02T18:46:03Z

README.md

+### Detected 6 changes to dependencies in Poetry lockfile
+
+**Base commit hash (new poetry.lock):** `f4e6ca0f4d67d9bb3f8ab43a89ceca2d0d2be7a1`
+**Target commit hash (old poetry.lock):** `a86b84f85d0bb2bf2fca6d6e8c58f2ce6f9e393c`
+**diff-poetry-lock version:** `1.0.1`
+
+Added **pydantic** (1.10.6)
+Added **requests-mock** (1.10.0)
+Added **six** (1.16.0)
+Added **tomli** (2.0.1)
+Added **typing-extensions** (4.5.0)
+Updated **urllib3** (1.26.14 -> 1.26.15)
+
+*(5 added, 0 removed, 1 updated, 4 not changed)*


suggestion: If you take my above code suggestion:

Suggested change

### Detected 6 changes to dependencies in Poetry lockfile

**Base commit hash (new poetry.lock):** `f4e6ca0f4d67d9bb3f8ab43a89ceca2d0d2be7a1`

**Target commit hash (old poetry.lock):** `a86b84f85d0bb2bf2fca6d6e8c58f2ce6f9e393c`

**diff-poetry-lock version:** `1.0.1`

Added **pydantic** (1.10.6)

Added **requests-mock** (1.10.0)

Added **six** (1.16.0)

Added **tomli** (2.0.1)

Added **typing-extensions** (4.5.0)

Updated **urllib3** (1.26.14 -> 1.26.15)

*(5 added, 0 removed, 1 updated, 4 not changed)*

### Detected 6 changes to dependencies in Poetry lockfile

From base f4e6ca0f4d67d9bb3f8ab43a89ceca2d0d2be7a1 to target a86b84f85d0bb2bf2fca6d6e8c58f2ce6f9e393c:

Added **pydantic** (1.10.6)

Added **requests-mock** (1.10.0)

Added **six** (1.16.0)

Added **tomli** (2.0.1)

Added **typing-extensions** (4.5.0)

Updated **urllib3** (1.26.14 -> 1.26.15)

*(5 added, 0 removed, 1 updated, 4 not changed)*

<small>Generated by diff-poetry-lock 1.0.1</small>\n\n

Remember, GH will autolink the hashes and shorten to just 8 chars on display.

colindean · 2026-03-02T18:49:49Z

diff_poetry_lock/github.py

+        try:
+            r = self.session.get(
+                f"{self.s.api_url}/repos/{self.s.repository}/commits/{ref}",
+                headers={"Authorization": f"token {self.s.token}", "Accept": "application/vnd.github+json"},


note: We should extract this authorization header generation to a method. It's copypasted all over this class.

Extracted the headers to a function

colindean · 2026-03-02T18:52:05Z

diff_poetry_lock/test/test_poetry_diff.py

+    **Base commit hash (new poetry.lock):** `new-sha`
+    **Target commit hash (old poetry.lock):** `old-sha`
+    **diff-poetry-lock version:** `test-version`
+
    Removed **pydantic** (1.10.6)
    Removed **typing-extensions** (4.5.0)
    Updated **urllib3** (1.26.15 -> 1.26.14)

    *(0 added, 2 removed, 1 updated, 4 not changed)*"""


todo: This will have to change if you take my suggestion below.

colindean · 2026-03-02T18:52:22Z

diff_poetry_lock/test/test_poetry_diff.py

    ### Detected 3 changes to dependencies in Poetry lockfile

+    **Base commit hash (new poetry.lock):** `new-sha`
+    **Target commit hash (old poetry.lock):** `old-sha`
+    **diff-poetry-lock version:** `test-version`
+
    Added **pydantic** (1.10.6)
    Added **typing-extensions** (4.5.0)
    Updated **urllib3** (1.26.14 -> 1.26.15)

    *(2 added, 0 removed, 1 updated, 4 not changed)*"""


todo: This will have to change if you take my suggestion below.

Consider extracting this example to a variable since of duplicating it, especially if it's not different from above.

1- Modified commit hash lookup to be done in the get_file api call 2- Altered diff output format 3- Wrapped the api headers into a function to be reused instead of the headers dict Signed-off-by: banginji <ban.ginji@users.noreply.github.com>

Signed-off-by: banginji <ban.ginji@users.noreply.github.com>

banginji added 3 commits February 25, 2026 23:16

Added additional metadata to diff output

bdf76be

Added in current and new commit hashes Added the current version of the tool Signed-off-by: banginji <ban.ginji@gmail.com>

Fixed a ruff format error

45ca911

Signed-off-by: banginji <ban.ginji@gmail.com>

Fixed ruff check error in tests

ce0aac8

Signed-off-by: banginji <ban.ginji@gmail.com>

banginji requested a review from colindean February 26, 2026 05:31

colindean requested changes Mar 2, 2026

View reviewed changes

banginji added 2 commits March 2, 2026 19:42

Made fixes based on review comments

7222aee

1- Modified commit hash lookup to be done in the get_file api call 2- Altered diff output format 3- Wrapped the api headers into a function to be reused instead of the headers dict Signed-off-by: banginji <ban.ginji@users.noreply.github.com>

Fixed an error pointed out by ruff format

a3170a1

Signed-off-by: banginji <ban.ginji@users.noreply.github.com>

+change_count = len(added + removed + updated)
+comment = f"### Detected {change_count} changes to dependencies in Poetry lockfile\n\n"
+if base_commit_hash and target_commit_hash:
+        # let GitHub auto-link the hashes
+        comment += f"From base {base_commit_hash} to target {target_commit_hash}:\n\n"
+summary_lines = [p.summary_line() for p in added + removed + updated]
+comment += "\n".join(summary_lines)
+comment += (
+f"\n\n*({len(added)} added, {len(removed)} removed, {len(updated)} updated, {len(not_changed)} not changed)*"
+)
+if diff_poetry_lock_version:
+    comment += f"<small>Generated by diff-poetry-lock {diff_poetry_lock_version}</small>\n\n"

		base_commit_hash = api.resolve_commit_hash(settings.ref)
		target_commit_hash = api.resolve_commit_hash(settings.base_ref)

Conversation

banginji commented Feb 26, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants