Skip to content

Conversation

@ferringb
Copy link

Review this after #29 lands. It's directly built on top of that.

See issue #28 ; the commit "added, removed" etc data wasn't coming through, and author name and email parsing was generally horked. Both were regex issues which I either replaced or rewrote.

For the name and email parsing, this is obvious; check the commit in this PR.

Note there is a subtle fix in here- for the first file mutation data, diff-tree will not properly reference the parent since there is no parent. IE, it gives back no changed files even if obviously the first commit adds files. The solution is to force it to diff against the hardcoded SHA for repository initialization.

Don't ask me how I know of this gotcha, but it's bit me in the ass before, thus I noticed it.

Other points of note:

  • -M was added so renames are actually detected.
  • the git function was allowing non-zero exit code from git- bad calls. This is why the ref deletion event was exploding further down the stack for example. I changed this to explode if a git call fails.
  • added warnings for diff-tree results that should be impossible for this code usage.

Signed-off-by: Brian Harring <[email protected]>
Fixes
* added boolean deleted and created fields per v3 spec.  These indicate
  if it's a ref deletion, new ref created, etc.

* fixed the exception for deletion events

* forced data annotation via dataclass, thus documenting the event structure.
  I did this explicitly because there are no tests and I had to rework a lot
  of this, so might as well use a dataclass to ensure I didn't drop any event
  data.  This is not runtime enforcement however.

* fixed the comparison urls sent for a ref creation event.

This is larger than intended, but the data validation I had to shove in to
verify I didn't break anything.  The result is simpler/cleaner however.

Signed-off-by: Brian Harring <[email protected]>
This is what was masking the failure in handling deletion
events and certain creation events.

Signed-off-by: Brian Harring <[email protected]>
See pr metajack#27 mostly addressed this, but this version is more
paranoid.  When this lands, metajack#27 can be closed out.

Signed-off-by: Brian Harring <[email protected]>
Note: this actually fails on the first commit in history;
diff-tree is against the parent.  It's edge case, but it exists.

This is a seperated PR from metajack#29 for review reasons.  When this and that
lands, issue metajack#28 can be closed.

Signed-off-by: Brian Harring <[email protected]>
@ferringb ferringb force-pushed the fix-commit-data branch 2 times, most recently from 8120793 to d35cbc3 Compare November 12, 2025 11:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant