Skip to content

Commit

Permalink
Fix type of commit for create_file
Browse files Browse the repository at this point in the history
The `commit` key of `create_file`'s result currently uses
`github.Commit.Commit`, but that does not match the data model of the
endpoint: while ill documented on docs.github.com, looking at the
published schema (via https://github.com/github/rest-api-description
or more readably https://github.com/octokit/types.ts which is
automatically generated from the former) `file-commit.commit` looks
significantly more like a `git-commit` than a `commit`:

* `commit`'s author and committer are git*hub* objects
  (`nullable-simple-user`), while `file-commit.commit`'s are git
  objects (triplets of name, email, and date)
* `commit` embeds the tree, and message in a sub-object, while
  `file-commit.commit` has them at the toplevel
* `file-commit.commit` has no `stats` or `files` properties, or
  `comments_url`

In all of these, `file-commit.commit` matches `git-commit` exactly.

The only real divergence between `git-commit` and `file-commit.commit`
is that the schema indicates all of `file-commit.commit`'s properties
are optional, but I think it's a case of the endpoint being
under-specified and the properties *not* being marked required, rather
than them being explicitly optional. See
github/rest-api-description#650 for an issue
on that subject with more information.
  • Loading branch information
xmo-odoo committed Jun 1, 2023
1 parent 9f52e94 commit 405e668
Show file tree
Hide file tree
Showing 3 changed files with 27 additions and 4 deletions.
4 changes: 2 additions & 2 deletions github/Repository.py
Original file line number Diff line number Diff line change
Expand Up @@ -2188,7 +2188,7 @@ def create_file(
:param author: InputGitAuthor, (optional), if omitted this will be filled in with committer information. If passed, you must specify both a name and email.
:rtype: {
'content': :class:`ContentFile <github.ContentFile.ContentFile>`:,
'commit': :class:`Commit <github.Commit.Commit>`}
'commit': :class:`Commit <github.GitCommit.GitCommit>`}
"""
assert isinstance(path, str)
assert isinstance(message, str)
Expand Down Expand Up @@ -2223,7 +2223,7 @@ def create_file(
"content": github.ContentFile.ContentFile(
self._requester, headers, data["content"], completed=False
),
"commit": github.Commit.Commit(
"commit": github.GitCommit.GitCommit(
self._requester, headers, data["commit"], completed=True
),
}
Expand Down
2 changes: 1 addition & 1 deletion github/Repository.pyi
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ class Repository(CompletableGithubObject):
branch: Union[str, _NotSetType] = ...,
committer: Union[InputGitAuthor, _NotSetType] = ...,
author: Union[InputGitAuthor, _NotSetType] = ...,
) -> Dict[str, Union[ContentFile, Commit]]: ...
) -> Dict[str, Union[ContentFile, GitCommit]]: ...
def create_git_blob(self, content: str, encoding: str) -> GitBlob: ...
def create_git_commit(
self,
Expand Down
25 changes: 24 additions & 1 deletion tests/Repository.py
Original file line number Diff line number Diff line change
Expand Up @@ -1412,14 +1412,37 @@ def testCreateFile(self):
"Enix Yu", "enix223@163.com", "2016-01-15T16:13:30+12:00"
)
self.assertEqual(repr(author), 'InputGitAuthor(name="Enix Yu")')
self.repo.create_file(
result = self.repo.create_file(
path=newFile,
message="Create file for testCreateFile",
content=content,
branch="master",
committer=author,
author=author,
)
commit = result["commit"]
raw_keys = set(commit.raw_data.keys())
self.assertLessEqual(
{
"sha",
"parents",
"tree",
"author",
"committer",
"url",
"html_url",
# missing from replay data but documented in schema
# "node_id",
# "message",
# "verification",
},
raw_keys,
"raw data should match (be a subset of) git-commit keys",
)
self.assertFalse(
{"comments_url", "commit", "stats", "files"} & raw_keys,
"characteristic attributes of commit should not be in the raw data",
)

def testUpdateFile(self):
updateFile = "doc/testCreateUpdateDeleteFile.md"
Expand Down

0 comments on commit 405e668

Please sign in to comment.