Sitelet https://web.archive.org/web/20200916181051/https://github.com/nodegit/nodegit/pull/1509
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

Fix behavior of Commit#parent #1509

Merged
merged 1 commit into from Jan 21, 2020

Conversation

@tniessen
Copy link
Contributor

tniessen commented Jun 6, 2018

Commit#parent did not assign the repo property so subsequent calls which relied on this property (such as Commit#getTree) failed. This change works around this problem by imitating what LookupWrapper does for Commit.lookup, that is, it wraps the actual call in a new function and manually assigns the repo property.

This commit also adds a test case for the corrected behavior.

@tniessen
Copy link
Contributor Author

tniessen commented Jun 22, 2018

lib/commit.js Outdated Show resolved Hide resolved
@tniessen
Copy link
Contributor Author

tniessen commented Jul 22, 2018

@implausible Sorry for the late reply. Is the new commit better?

@implausible implausible force-pushed the nodegit:master branch from c345989 to c67b436 Aug 26, 2019
@tniessen
Copy link
Contributor Author

tniessen commented Jan 19, 2020

Ping @implausible, it's been far more than a year, is this still a helpful PR? If not, please close it.

Commit#parent did not assign the repo property so subsequent calls which
relied on this property (such as Commit#getTree) failed. This change
works around this problem by imitating what LookupWrapper does for
Commit.lookup, that is, it wraps the actual call in a new function and
manually assigns the repo property.
@tniessen tniessen force-pushed the tniessen:fix-commit-parent-no-repo-prop branch from a84df76 to e3bb744 Jan 19, 2020
@tbranyen tbranyen dismissed implausible’s stale review Jan 19, 2020

It has been resolved

@implausible implausible merged commit cf10bce into nodegit:master Jan 21, 2020
12 checks passed
12 checks passed
*nix Tests (8, ubuntu-16.04)
Details
*nix Tests (8, macOS-10.14) *nix Tests (8, macOS-10.14)
Details
*nix Tests (10, ubuntu-16.04)
Details
*nix Tests (10, macOS-10.14) *nix Tests (10, macOS-10.14)
Details
*nix Tests (12, ubuntu-16.04)
Details
*nix Tests (12, macOS-10.14) *nix Tests (12, macOS-10.14)
Details
Windows Tests (8, x86) Windows Tests (8, x86)
Details
Windows Tests (8, x64) Windows Tests (8, x64)
Details
Windows Tests (10, x86) Windows Tests (10, x86)
Details
Windows Tests (10, x64) Windows Tests (10, x64)
Details
Windows Tests (12, x86) Windows Tests (12, x86)
Details
Windows Tests (12, x64) Windows Tests (12, x64)
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.