diff --git a/library_generation/generate_pr_description.py b/library_generation/generate_pr_description.py index 92256c0a2f..e4852b6eab 100644 --- a/library_generation/generate_pr_description.py +++ b/library_generation/generate_pr_description.py @@ -19,7 +19,10 @@ from git import Commit, Repo from library_generation.model.generation_config import GenerationConfig from library_generation.utils.proto_path_utils import find_versioned_proto_path -from library_generation.utils.commit_message_formatter import format_commit_message +from library_generation.utils.commit_message_formatter import ( + format_commit_message, + commit_link, +) from library_generation.utils.commit_message_formatter import wrap_override_commit EMPTY_MESSAGE = "" @@ -58,8 +61,8 @@ def generate_pr_descriptions( paths = config.get_proto_path_to_library_name() description = get_commit_messages( repo_url=repo_url, - current_commit=config.googleapis_commitish, - baseline_commit=baseline_commit, + current_commit_sha=config.googleapis_commitish, + baseline_commit_sha=baseline_commit, paths=paths, is_monorepo=config.is_monorepo(), ) @@ -76,8 +79,8 @@ def generate_pr_descriptions( def get_commit_messages( repo_url: str, - current_commit: str, - baseline_commit: str, + current_commit_sha: str, + baseline_commit_sha: str, paths: Dict[str, str], is_monorepo: bool, ) -> str: @@ -88,9 +91,9 @@ def get_commit_messages( Note that baseline_commit should be an ancestor of latest_commit. :param repo_url: the url of the repository. - :param current_commit: the newest commit to be considered in + :param current_commit_sha: the newest commit to be considered in selecting commit message. - :param baseline_commit: the oldest commit to be considered in + :param baseline_commit_sha: the oldest commit to be considered in selecting commit message. This commit should be an ancestor of :param paths: a mapping from file paths to library_name. :param is_monorepo: whether to generate commit messages in a monorepo. @@ -102,17 +105,19 @@ def get_commit_messages( shutil.rmtree(tmp_dir, ignore_errors=True) os.mkdir(tmp_dir) repo = Repo.clone_from(repo_url, tmp_dir) - commit = repo.commit(current_commit) - current_commit_time = __get_commit_timestamp(commit) - baseline_commit_time = __get_commit_timestamp(repo.commit(baseline_commit)) + current_commit = repo.commit(current_commit_sha) + baseline_commit = repo.commit(baseline_commit_sha) + current_commit_time = __get_commit_timestamp(current_commit) + baseline_commit_time = __get_commit_timestamp(baseline_commit) if current_commit_time <= baseline_commit_time: raise ValueError( - f"current_commit ({current_commit[:7]}, committed on " + f"current_commit ({current_commit_sha[:7]}, committed on " f"{current_commit_time}) should be newer than baseline_commit " - f"({baseline_commit[:7]}, committed on {baseline_commit_time})." + f"({baseline_commit_sha[:7]}, committed on {baseline_commit_time})." ) qualified_commits = {} - while str(commit.hexsha) != baseline_commit: + commit = current_commit + while str(commit.hexsha) != baseline_commit_sha: commit_and_name = __filter_qualified_commit(paths=paths, commit=commit) if commit_and_name != (): qualified_commits[commit_and_name[0]] = commit_and_name[1] @@ -151,20 +156,16 @@ def __filter_qualified_commit(paths: Dict[str, str], commit: Commit) -> (Commit, def __combine_commit_messages( - current_commit: str, - baseline_commit: str, + current_commit: Commit, + baseline_commit: Commit, commits: Dict[Commit, str], is_monorepo: bool, ) -> str: messages = [ - f"This pull request is generated with proto changes between googleapis commit {baseline_commit} (exclusive) and {current_commit} (inclusive).", - "Qualified commits are:", + f"This pull request is generated with proto changes between {commit_link(baseline_commit)} (exclusive) " + f"and {commit_link(current_commit)} (inclusive).", + "", ] - for commit in commits: - short_sha = commit.hexsha[:7] - messages.append( - f"[googleapis/googleapis@{short_sha}](https://github.com/googleapis/googleapis/commit/{commit.hexsha})" - ) messages.extend( wrap_override_commit( diff --git a/library_generation/test/resources/integration/google-cloud-java/pr-description-golden.txt b/library_generation/test/resources/integration/google-cloud-java/pr-description-golden.txt index 0787f81113..cb855dcd52 100644 --- a/library_generation/test/resources/integration/google-cloud-java/pr-description-golden.txt +++ b/library_generation/test/resources/integration/google-cloud-java/pr-description-golden.txt @@ -1,23 +1,19 @@ -This pull request is generated with proto changes between googleapis commit a17d4caf184b050d50cacf2b0d579ce72c31ce74 (exclusive) and 4ce0ff67a3d4509be641cbe47a35844ddc1268fc (inclusive). -Qualified commits are: -[googleapis/googleapis@7659dd2](https://github.com/googleapis/googleapis/commit/7659dd2244563fd902b681bdcadbe5385b8d3462) -[googleapis/googleapis@05d889e](https://github.com/googleapis/googleapis/commit/05d889e7dfe087fc2ddc9de9579f01d4e1c2f35e) -[googleapis/googleapis@aa16fda](https://github.com/googleapis/googleapis/commit/aa16fdad909bc33e2d4ff04cfde56a46d0e52b13) -[googleapis/googleapis@0733fdb](https://github.com/googleapis/googleapis/commit/0733fdb5f745192f9f3c95f8d08039286567cbcc) -[googleapis/googleapis@9e35c62](https://github.com/googleapis/googleapis/commit/9e35c620157d7b11cb5b2e5d0249c5caaee824f3) -[googleapis/googleapis@36dedd0](https://github.com/googleapis/googleapis/commit/36dedd0d9020c19d1c8259003c2fe9656ada7471) +This pull request is generated with proto changes between [googleapis/googleapis@a17d4ca](https://github.com/googleapis/googleapis/commit/a17d4caf184b050d50cacf2b0d579ce72c31ce74) (exclusive) and [googleapis/googleapis@4ce0ff6](https://github.com/googleapis/googleapis/commit/4ce0ff67a3d4509be641cbe47a35844ddc1268fc) (inclusive). + BEGIN_COMMIT_OVERRIDE BEGIN_NESTED_COMMIT docs: [cloudcontrolspartner] update documentation URL PiperOrigin-RevId: 612723053 +Source Link: [googleapis/googleapis@7659dd2](https://github.com/googleapis/googleapis/commit/7659dd2244563fd902b681bdcadbe5385b8d3462) END_NESTED_COMMIT BEGIN_NESTED_COMMIT feat: [cloudcontrolspartner] added CloudControlsPartner API PiperOrigin-RevId: 612632640 +Source Link: [googleapis/googleapis@05d889e](https://github.com/googleapis/googleapis/commit/05d889e7dfe087fc2ddc9de9579f01d4e1c2f35e) END_NESTED_COMMIT BEGIN_NESTED_COMMIT feat: [alloydb] support for obtaining the public IP address of an Instance @@ -28,6 +24,7 @@ docs: [alloydb] clarified read pool config is for read pool type instances PiperOrigin-RevId: 610475013 +Source Link: [googleapis/googleapis@aa16fda](https://github.com/googleapis/googleapis/commit/aa16fdad909bc33e2d4ff04cfde56a46d0e52b13) END_NESTED_COMMIT BEGIN_NESTED_COMMIT feat: [alloydb] support for obtaining the public IP address of an Instance @@ -35,6 +32,7 @@ feat: [alloydb] support for getting PSC DNS name from the GetConnectionInfo API PiperOrigin-RevId: 610415824 +Source Link: [googleapis/googleapis@0733fdb](https://github.com/googleapis/googleapis/commit/0733fdb5f745192f9f3c95f8d08039286567cbcc) END_NESTED_COMMIT BEGIN_NESTED_COMMIT docs: [cloudcontrolspartner] Updated comment for method `ListCustomers` in service `CloudControlsPartnerCore` @@ -44,11 +42,13 @@ docs: [cloudcontrolspartner] Updated documentation URL PiperOrigin-RevId: 609026905 +Source Link: [googleapis/googleapis@9e35c62](https://github.com/googleapis/googleapis/commit/9e35c620157d7b11cb5b2e5d0249c5caaee824f3) END_NESTED_COMMIT BEGIN_NESTED_COMMIT feat: [cloudcontrolspartner] added CloudControlsPartner API PiperOrigin-RevId: 606720708 +Source Link: [googleapis/googleapis@36dedd0](https://github.com/googleapis/googleapis/commit/36dedd0d9020c19d1c8259003c2fe9656ada7471) END_NESTED_COMMIT END_COMMIT_OVERRIDE \ No newline at end of file diff --git a/library_generation/test/utils/commit_message_formatter_unit_tests.py b/library_generation/test/utils/commit_message_formatter_unit_tests.py index a13ffc229d..0148214dfb 100644 --- a/library_generation/test/utils/commit_message_formatter_unit_tests.py +++ b/library_generation/test/utils/commit_message_formatter_unit_tests.py @@ -14,7 +14,10 @@ import unittest from unittest.mock import patch -from library_generation.utils.commit_message_formatter import format_commit_message +from library_generation.utils.commit_message_formatter import ( + format_commit_message, + commit_link, +) from library_generation.utils.commit_message_formatter import wrap_nested_commit from library_generation.utils.commit_message_formatter import wrap_override_commit @@ -26,12 +29,14 @@ def test_format_commit_message_should_add_library_name_for_conventional_commit( with patch("git.Commit") as mock_commit: commit = mock_commit.return_value commit.message = "feat: a commit message\nPiperOrigin-RevId: 123456" + commit.hexsha = "1234567abcdefg" commits = {commit: "example_library"} self.assertEqual( [ "BEGIN_NESTED_COMMIT", "feat: [example_library] a commit message", "PiperOrigin-RevId: 123456", + "Source Link: [googleapis/googleapis@1234567](https://github.com/googleapis/googleapis/commit/1234567abcdefg)", "END_NESTED_COMMIT", ], format_commit_message(commits, True), @@ -43,6 +48,7 @@ def test_format_commit_message_should_add_library_name_for_mutliline_conventiona with patch("git.Commit") as mock_commit: commit = mock_commit.return_value commit.message = "feat: a commit message\nfix: an another commit message\nPiperOrigin-RevId: 123456" + commit.hexsha = "1234567abcdefg" commits = {commit: "example_library"} self.assertEqual( [ @@ -50,6 +56,7 @@ def test_format_commit_message_should_add_library_name_for_mutliline_conventiona "feat: [example_library] a commit message", "fix: [example_library] an another commit message", "PiperOrigin-RevId: 123456", + "Source Link: [googleapis/googleapis@1234567](https://github.com/googleapis/googleapis/commit/1234567abcdefg)", "END_NESTED_COMMIT", ], format_commit_message(commits, True), @@ -61,11 +68,13 @@ def test_format_commit_message_should_not_add_library_name_for_nonconvnentional_ with patch("git.Commit") as mock_commit: commit = mock_commit.return_value commit.message = "PiperOrigin-RevId: 123456" + commit.hexsha = "1234567abcdefg" commits = {commit: "example_library"} self.assertEqual( [ "BEGIN_NESTED_COMMIT", "PiperOrigin-RevId: 123456", + "Source Link: [googleapis/googleapis@1234567](https://github.com/googleapis/googleapis/commit/1234567abcdefg)", "END_NESTED_COMMIT", ], format_commit_message(commits, True), @@ -75,12 +84,14 @@ def test_format_commit_message_should_not_add_library_name_if_not_monorepo(self) with patch("git.Commit") as mock_commit: commit = mock_commit.return_value commit.message = "feat: a commit message\nPiperOrigin-RevId: 123456" + commit.hexsha = "1234567abcdefg" commits = {commit: "example_library"} self.assertEqual( [ "BEGIN_NESTED_COMMIT", "feat: a commit message", "PiperOrigin-RevId: 123456", + "Source Link: [googleapis/googleapis@1234567](https://github.com/googleapis/googleapis/commit/1234567abcdefg)", "END_NESTED_COMMIT", ], format_commit_message(commits, False), @@ -92,6 +103,7 @@ def test_format_commit_message_should_not_add_library_name_for_multiline_commit_ with patch("git.Commit") as mock_commit: commit = mock_commit.return_value commit.message = "feat: a commit message\nfix: an another commit message\nPiperOrigin-RevId: 123456" + commit.hexsha = "1234567abcdefg" commits = {commit: "example_library"} self.assertEqual( [ @@ -99,22 +111,27 @@ def test_format_commit_message_should_not_add_library_name_for_multiline_commit_ "feat: a commit message", "fix: an another commit message", "PiperOrigin-RevId: 123456", + "Source Link: [googleapis/googleapis@1234567](https://github.com/googleapis/googleapis/commit/1234567abcdefg)", "END_NESTED_COMMIT", ], format_commit_message(commits, False), ) def test_wrap_nested_commit_success(self): - messages = ["a commit message", "another message"] - self.assertEqual( - [ - "BEGIN_NESTED_COMMIT", - "a commit message", - "another message", - "END_NESTED_COMMIT", - ], - wrap_nested_commit(messages), - ) + with patch("git.Commit") as mock_commit: + commit = mock_commit.return_value + commit.hexsha = "1234567abcdefg" + messages = ["a commit message", "another message"] + self.assertEqual( + [ + "BEGIN_NESTED_COMMIT", + "a commit message", + "another message", + "Source Link: [googleapis/googleapis@1234567](https://github.com/googleapis/googleapis/commit/1234567abcdefg)", + "END_NESTED_COMMIT", + ], + wrap_nested_commit(commit, messages), + ) def test_wrap_override_commit_success(self): messages = ["a commit message", "another message"] @@ -127,3 +144,12 @@ def test_wrap_override_commit_success(self): ], wrap_override_commit(messages), ) + + def test_commit_link_success(self): + with patch("git.Commit") as mock_commit: + commit = mock_commit.return_value + commit.hexsha = "1234567abcdefg" + self.assertEqual( + "[googleapis/googleapis@1234567](https://github.com/googleapis/googleapis/commit/1234567abcdefg)", + commit_link(commit), + ) diff --git a/library_generation/utils/commit_message_formatter.py b/library_generation/utils/commit_message_formatter.py index d65eabecaf..85bfbc8036 100644 --- a/library_generation/utils/commit_message_formatter.py +++ b/library_generation/utils/commit_message_formatter.py @@ -47,19 +47,21 @@ def format_commit_message(commits: Dict[Commit, str], is_monorepo: bool) -> List messages.append(formatted_message) else: messages.append(message_line) - all_commits.extend(wrap_nested_commit(messages)) + all_commits.extend(wrap_nested_commit(commit, messages)) return all_commits -def wrap_nested_commit(messages: List[str]) -> List[str]: +def wrap_nested_commit(commit: Commit, messages: List[str]) -> List[str]: """ Wrap message between `BEGIN_NESTED_COMMIT` and `BEGIN_NESTED_COMMIT`. + :param commit: a GitHub commit. :param messages: a (multi-line) commit message, one line per item. :return: wrapped messages. """ result = ["BEGIN_NESTED_COMMIT"] result.extend(messages) + result.append(f"Source Link: {commit_link(commit)}") result.append("END_NESTED_COMMIT") return result @@ -75,3 +77,14 @@ def wrap_override_commit(messages: List[str]) -> List[str]: result.extend(messages) result.append("END_COMMIT_OVERRIDE") return result + + +def commit_link(commit: Commit) -> str: + """ + Create a link to the commit in Markdown format. + + :param commit: a GitHub commit. + :return: a link in Markdown format. + """ + short_sha = commit.hexsha[:7] + return f"[googleapis/googleapis@{short_sha}](https://github.com/googleapis/googleapis/commit/{commit.hexsha})"