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

Update stale comments relating to object scanning and uploading #5163

Merged
merged 2 commits into from Oct 31, 2022

Conversation

chrisd8088
Copy link
Contributor

This PR updates several stale comments that relate to scanning Git blob objects to find valid Git LFS pointers and to validating that objects have corresponding local file copies before uploading them.

In commit e3fcde7 of PR #3236 the ObjectScanner type was updated to use an internal ObjectDatabase from the github.com/git-lfs/gitobj package rather than invoking an external git cat-file --batch command. However, the comments for the catFileBatch() and runCatFileBatch() functions were not updated at the same time and still state that they cause a git cat-file --batch command to run, so we update them now.

And in commit 338ab40 of PR #1812 the prepareUpload() function was refactored so that it no longer checked whether an object to be pushed had a local file copy or not and skipped it if it did not. However, the comment which stated that the function skipped any objects which did not have local file copies was not updated, so we now revise that comment and add one in the uploadTransfer() function.

In commit 338ab40 of PR git-lfs#1812 the
prepareUpload() function was refactored so that it no longer checked
whether an object to be pushed had a local file copy or not and skipped it
if it did not.

(That functionality already existed in the uploadTransfer() function
in that it called the ensureFile() function before returning a
tq.Transfer object.  Note that these functions were in the
commands/commands.go source file at the time.  The uploadTransfer()
function is called for each pointer blob object by uploadPointers().)

However, commit 338ab40 did not
update the comment in prepareUpload() which stated that the function
skipped any objects which did not have local file copies, so we now
revise that comment and add one in the uploadTransfer() function.
In commit e3fcde7 of PR git-lfs#3236 the
ObjectScanner type was updated to use an internal ObjectDatabase from
the github.com/git-lfs/gitobj package rather than invoking an external
"git cat-file --batch" command.

However, the comments for the catFileBatch() and runCatFileBatch()
functions were not updated at the same time and still state that they
cause a "git cat-file --batch" command to run, so we update them now.
@chrisd8088 chrisd8088 requested a review from a team as a code owner October 31, 2022 19:32
@chrisd8088 chrisd8088 merged commit c80090c into git-lfs:main Oct 31, 2022
@chrisd8088 chrisd8088 deleted the update-stale-comments branch October 31, 2022 21:00
chrisd8088 added a commit to chrisd8088/git-lfs that referenced this pull request Nov 1, 2022
In commit 201b8ba of PR git-lfs#5163 we
updated several comments relating to the change from the use of the
"git cat-file --batch" command to an internal ObjectDatabase provided
by the github.com/git-lfs/gitobj package.

This change was originally made in e3fcde7
of PR git-lfs#3236, but various comments were not updated at the same time.
In PR git-lfs#5163 we missed two of those comments, for the NewObjectScanner()
and catFileBatchTree() functions, so we update those now as well.
chrisd8088 added a commit to chrisd8088/git-lfs that referenced this pull request Nov 1, 2022
In commit 201b8ba of PR git-lfs#5163 we
updated several comments relating to the change from the use of the
"git cat-file --batch" command to an internal ObjectDatabase provided
by the github.com/git-lfs/gitobj package.

This change was originally made in e3fcde7
of PR git-lfs#3236, but various comments were not updated at the same time.
In PR git-lfs#5163 we missed two of those comments, for the NewObjectScanner()
and catFileBatchTree() functions, so we update those now as well.
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.

None yet

2 participants