Skip to content

Adding unit test for tree attribute of GitCommit#668

Merged
jasonwhite merged 2 commits intoPyGithub:masterfrom
sfdye:add-tree-to-commit
Dec 10, 2017
Merged

Adding unit test for tree attribute of GitCommit#668
jasonwhite merged 2 commits intoPyGithub:masterfrom
sfdye:add-tree-to-commit

Conversation

@sfdye
Copy link
Copy Markdown
Member

@sfdye sfdye commented Dec 8, 2017

Adding a simple unit test for tree attribute for GitCommit.

github/Commit.py Outdated
if "url" in attributes: # pragma no branch
self._url = self._makeStringAttribute(attributes["url"])
if "tree" in attributes: # pragma no branch
self._tree = self._makeClassAttribute(Commit, attributes["tree"])
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This doesn't really make sense. The tree is not a commit. The JSON also doesn't quite provide enough information to construct a GitTree object either (it's missing the subtree elements).

It might make more sense to expose a property called tree_sha instead. Then, it could be used in conjunction with Repository.get_git_tree().

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Seems like I was confusing the Commit with GitCommit, actually the tree attribute is already. So I will just add the missing unit test for this PR.

@sfdye sfdye changed the title Add tree attribute to Commit [Test] tree attribute for GitCommit Dec 9, 2017
@sfdye sfdye changed the title [Test] tree attribute for GitCommit Adding unit test for tree attribute of GitCommit Dec 9, 2017
@sfdye
Copy link
Copy Markdown
Member Author

sfdye commented Dec 9, 2017

@jasonwhite how about now?

Copy link
Copy Markdown
Collaborator

@jasonwhite jasonwhite left a comment

Choose a reason for hiding this comment

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

Looks good. I had forgotten about the commit attribute. This makes more sense. Thanks!

@jasonwhite jasonwhite merged commit e5bfdbe into PyGithub:master Dec 10, 2017
@sfdye sfdye deleted the add-tree-to-commit branch December 11, 2017 03:54
@sfdye
Copy link
Copy Markdown
Member Author

sfdye commented Dec 11, 2017

Thanks for pointing out my mistake earlier 😃

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.

2 participants