Skip to content

Commit

Permalink
feat: Use original commit message on one commit updates (#1)
Browse files Browse the repository at this point in the history
If a template update only applies one commit to the target repository,
the original commit message and body can be applied without
modifications
instead of the summary with all changes for bulk edits.

This change introduces a detection if the template update only contains
one commit and uses the
original message accordingly.

This will allow maintainers to update repositories in smaller
increments, such that the commit history may partially mirror the commit
history of the template.

---------

Co-authored-by: Jan-Frederik Schmidt <jan-frederik.schmidt@vorausrobotik.com>
  • Loading branch information
DS424 and g3n35i5 authored Jan 8, 2024
1 parent 61d6e64 commit f6471d3
Show file tree
Hide file tree
Showing 2 changed files with 99 additions and 15 deletions.
38 changes: 32 additions & 6 deletions src/voraus_template_updater/_update_projects.py
Original file line number Diff line number Diff line change
Expand Up @@ -180,14 +180,19 @@ def _update_project(

cruft.update(Path(local_repo.working_dir), checkout=project.template_branch)

template_commit_messages = _get_template_commit_messages(project, github_access_token)
local_repo.git.add(all=True)
local_repo.index.commit(PR_TITLE)
if len(template_commit_messages) == 1:
local_repo.index.commit(template_commit_messages[0])
else:
local_repo.index.commit(PR_TITLE)
local_repo.git.push("--set-upstream", "origin", branch)

pr_body = _get_pr_body(project, github_access_token)
pr_title = _get_pr_title(template_commit_messages)
pr_body = _get_pr_body(project, template_commit_messages)

pull_request = remote_repo.create_pull(
base=remote_repo.default_branch, head=branch.name, title=PR_TITLE, body=pr_body
base=remote_repo.default_branch, head=branch.name, title=pr_title, body=pr_body
)

_logger.info(
Expand All @@ -198,7 +203,7 @@ def _update_project(
return pull_request


def _get_pr_body(project: Project, github_access_token: str) -> str:
def _get_template_commit_messages(project: Project, github_access_token: str) -> List[str]:
with TemporaryDirectory() as tmp_path:
template_repo = _clone_repo(project.template_url, github_access_token, Path(tmp_path))

Expand All @@ -222,12 +227,33 @@ def _get_pr_body(project: Project, github_access_token: str) -> str:
r"\(#(\d+)\)", lambda match: f"([PR]({link.format(match.groups()[0])}))", message
)

return commit_messages


def _get_pr_title(template_commit_messages: List[str]) -> str:
if len(template_commit_messages) == 1:
message = template_commit_messages[0]
return message.strip().splitlines()[0]

return PR_TITLE


def _get_pr_body(project: Project, template_commit_messages: List[str]) -> str:
if len(template_commit_messages) == 1:
message = template_commit_messages[0]
lines = message.strip().splitlines()
if len(lines) > 1:
return "".join(message.strip().splitlines()[1:])
return ""

# Indent all lines after the first line of a commit message by two spaces
# This leads to nicer bullet points in the pull request body
commit_messages = ["\n ".join(commit_message.strip().splitlines()) for commit_message in commit_messages]
template_commit_messages = [
"\n ".join(commit_message.strip().splitlines()) for commit_message in template_commit_messages
]

# Construct a pull request message containing the PR header and a bullet point list of changes and their explanation
return PR_BODY_HEADER.format(project.template_branch) + "- " + "\n\n- ".join(commit_messages) + "\n"
return PR_BODY_HEADER.format(project.template_branch) + "- " + "\n\n- ".join(template_commit_messages) + "\n"


if __name__ == "__main__": # pragma: no cover
Expand Down
76 changes: 67 additions & 9 deletions tests/unit/test_update_projects.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@
_check_and_update_projects,
_clone_repo,
_get_cruft_config,
_get_pr_body,
_get_pr_title,
)

ORGANIZATION = "dummy-organization"
Expand Down Expand Up @@ -318,11 +320,13 @@ def test_up_to_date_project(
assert summary.projects[0].status == Status.UP_TO_DATE


@pytest.mark.parametrize("is_incremental_update", [True, False], ids=["Incremental Update", "Bulk Update"])
@patch("voraus_template_updater._update_projects.cruft.update")
@patch("voraus_template_updater._update_projects.cruft.check")
def test_update_project_success(
cruft_check_mock: MagicMock,
cruft_update_mock: MagicMock,
is_incremental_update: bool,
repo_mock: MagicMock,
cloned_repo_mocks: List[MagicMock],
cruft_config: CruftConfig,
Expand All @@ -343,7 +347,8 @@ def _create_commit_mock(i: int) -> MagicMock:
return commit_mock

cloned_template_repo.git.rev_parse.return_value = "newest_commit"
cloned_template_repo.iter_commits.return_value = [_create_commit_mock(i) for i in range(2)]
number_new_commits = 1 if is_incremental_update else 2
cloned_template_repo.iter_commits.return_value = [_create_commit_mock(i) for i in range(number_new_commits)]

with caplog.at_level(logging.INFO):
summary = _check_and_update_projects(ORGANIZATION)
Expand All @@ -359,18 +364,31 @@ def _create_commit_mock(i: int) -> MagicMock:
cloned_template_repo.iter_commits.assert_called_once_with(f"{cruft_config.commit}..newest_commit")

cloned_project_repo.git.add.assert_called_with(all=True)
cloned_project_repo.index.commit.assert_called_with(PR_TITLE)
if is_incremental_update:
cloned_project_repo.index.commit.assert_called_with(
"Commit title ([PR](some-template-url/pull/0))\n\nDescription 0\n"
)
else:
cloned_project_repo.index.commit.assert_called_with(PR_TITLE)
cloned_project_repo.git.push.assert_called_with("--set-upstream", "origin", branch_mock)

expected_pr_body = (
"Contains the following changes to get up-to-date with the newest version of the template's 'dev' branch."
"\n\n"
"- Commit title ([PR](some-template-url/pull/0))\n \n Description 0\n\n"
"- Commit title ([PR](some-template-url/pull/1))\n \n Description 1\n"
)
if is_incremental_update:
expected_pr_body = "Description 0"
else:
expected_pr_body = (
"Contains the following changes to get up-to-date with the newest version of the template's 'dev' branch."
"\n\n"
"- Commit title ([PR](some-template-url/pull/0))\n \n Description 0\n\n"
"- Commit title ([PR](some-template-url/pull/1))\n \n Description 1\n"
)

if is_incremental_update:
expected_pr_title = "Commit title ([PR](some-template-url/pull/0))"
else:
expected_pr_title = PR_TITLE

repo_mock.create_pull.assert_called_once_with(
base=repo_mock.default_branch, head=branch_mock.name, title=PR_TITLE, body=expected_pr_body
base=repo_mock.default_branch, head=branch_mock.name, title=expected_pr_title, body=expected_pr_body
)

assert caplog.record_tuples[1] == (
Expand All @@ -384,3 +402,43 @@ def _create_commit_mock(i: int) -> MagicMock:
assert summary.projects[0].status == Status.UPDATED_THIS_RUN
assert summary.projects[0].pull_request is not None
assert summary.projects[0].pull_request.html_url == "https://some-pr.com"


@pytest.mark.parametrize(
argnames=["commit_messages", "expected_title"],
argvalues=[
([], PR_TITLE),
(["Commit Title"], "Commit Title"),
(["Commit Title\n\nCommit Body\n"], "Commit Title"),
(["Commit Title\nCommit Body\n"], "Commit Title"),
(["Commit Title 1\n\nCommit Body 1\n", "Commit Title 2\n Commit body 1.\n"], PR_TITLE),
],
)
def test_get_pr_title(commit_messages: List[str], expected_title: str) -> None:
assert _get_pr_title(commit_messages) == expected_title


@pytest.mark.parametrize(
argnames=["commit_messages", "expected_body"],
argvalues=[
(
[],
"Contains the following changes to get up-to-date with the newest version of the template's 'dev' branch."
"\n\n- \n",
), # Empty commit should never happen. But this test shows, that we do not crash.
(["Commit Title"], ""),
(["Commit Title\n\nCommit Body"], "Commit Body"),
(["Commit Title\nCommit Body\n"], "Commit Body"),
(
["Commit Title", "Commit title\n\nCommit body.\n"],
"Contains the following changes to get up-to-date with the newest version of the template's 'dev' branch."
"\n\n"
"- Commit Title\n\n"
"- Commit title\n \n Commit body.\n",
),
],
)
def test_get_pr_body(commit_messages: List[str], expected_body: str) -> None:
project = MagicMock()
project.template_branch = "dev"
assert _get_pr_body(project, commit_messages) == expected_body

0 comments on commit f6471d3

Please sign in to comment.