-
Notifications
You must be signed in to change notification settings - Fork 53
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
feat: get PR description from googleapis commits #2531
Changes from 35 commits
eb93ac3
df8fe50
eb0da6b
9a1be4c
c1f556c
ccb1a21
057da91
da3fc4c
1e6ab5e
a1c786f
d128a2d
939c513
7079327
a08a866
b77aa1d
eecaba0
259c4ef
f09fb3b
5e1d85e
7e9be85
9f3bbca
8324776
cc9088e
334298b
aabd9c1
176871d
93e60de
d4d16f6
a279549
1946d64
be189c5
188a632
5d7c915
240038f
33df40b
cd7f639
1715714
6cf0045
b473438
2b507aa
2ce846a
c03d6dd
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,199 @@ | ||
#!/usr/bin/env python3 | ||
# Copyright 2024 Google LLC | ||
# | ||
# Licensed under the Apache License, Version 2.0 (the "License"); | ||
# you may not use this file except in compliance with the License. | ||
# You may obtain a copy of the License at | ||
# | ||
# http://www.apache.org/licenses/LICENSE-2.0 | ||
# | ||
# Unless required by applicable law or agreed to in writing, software | ||
# distributed under the License is distributed on an "AS IS" BASIS, | ||
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
# See the License for the specific language governing permissions and | ||
# limitations under the License. | ||
import os | ||
import shutil | ||
from typing import Dict | ||
|
||
import click | ||
from git import Commit, Repo | ||
from library_generation.model.generation_config import from_yaml | ||
from library_generation.utilities import find_versioned_proto_path | ||
from library_generation.utilities import get_file_paths | ||
|
||
|
||
@click.group(invoke_without_command=False) | ||
@click.pass_context | ||
@click.version_option(message="%(version)s") | ||
def main(ctx): | ||
pass | ||
|
||
|
||
@main.command() | ||
@click.option( | ||
"--generation-config-yaml", | ||
required=True, | ||
type=str, | ||
help=""" | ||
Path to generation_config.yaml that contains the metadata about | ||
library generation. | ||
The googleapis commit in the configuration is the latest commit, | ||
inclusively, from which the commit message is considered. | ||
""", | ||
) | ||
@click.option( | ||
"--baseline-commit", | ||
required=True, | ||
type=str, | ||
help=""" | ||
The baseline (oldest) commit, exclusively, from which the commit message is | ||
considered. | ||
This commit should be an ancestor of googleapis commit in configuration. | ||
""", | ||
) | ||
@click.option( | ||
"--repo-url", | ||
type=str, | ||
default="https://github.com/googleapis/googleapis.git", | ||
show_default=True, | ||
help=""" | ||
GitHub repository URL. | ||
""", | ||
) | ||
def generate( | ||
generation_config_yaml: str, | ||
repo_url: str, | ||
baseline_commit: str, | ||
) -> str: | ||
return generate_pr_descriptions( | ||
generation_config_yaml=generation_config_yaml, | ||
repo_url=repo_url, | ||
baseline_commit=baseline_commit, | ||
) | ||
|
||
|
||
def generate_pr_descriptions( | ||
generation_config_yaml: str, | ||
repo_url: str, | ||
baseline_commit: str, | ||
) -> str: | ||
config = from_yaml(generation_config_yaml) | ||
paths = get_file_paths(config) | ||
return __get_commit_messages( | ||
repo_url=repo_url, | ||
latest_commit=config.googleapis_commitish, | ||
baseline_commit=baseline_commit, | ||
paths=paths, | ||
generator_version=config.gapic_generator_version, | ||
is_monorepo=config.is_monorepo, | ||
) | ||
|
||
|
||
def __get_commit_messages( | ||
repo_url: str, | ||
latest_commit: str, | ||
baseline_commit: str, | ||
paths: Dict[str, str], | ||
generator_version: str, | ||
is_monorepo: bool, | ||
) -> str: | ||
""" | ||
Combine commit messages of a repository from latest_commit to | ||
baseline_commit. Only commits which change files in a pre-defined | ||
file paths will be considered. | ||
Note that baseline_commit should be an ancestor of latest_commit. | ||
|
||
:param repo_url: the url of the repository. | ||
:param latest_commit: the newest commit to be considered in | ||
selecting commit message. | ||
:param baseline_commit: 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 generator_version: the version of the generator. | ||
:param is_monorepo: whether to generate commit messages in a monorepo. | ||
:return: commit messages. | ||
""" | ||
tmp_dir = "/tmp/repo" | ||
shutil.rmtree(tmp_dir, ignore_errors=True) | ||
os.mkdir(tmp_dir) | ||
repo = Repo.clone_from(repo_url, tmp_dir) | ||
commit = repo.commit(latest_commit) | ||
qualified_commits = {} | ||
while str(commit.hexsha) != baseline_commit: | ||
commit_and_name = __filter_qualified_commit(paths=paths, commit=commit) | ||
if commit_and_name != (): | ||
qualified_commits[commit_and_name[0]] = commit_and_name[1] | ||
commit_parents = commit.parents | ||
if len(commit_parents) == 0: | ||
break | ||
commit = commit_parents[0] | ||
shutil.rmtree(tmp_dir, ignore_errors=True) | ||
return __combine_commit_messages( | ||
latest_commit=latest_commit, | ||
baseline_commit=baseline_commit, | ||
commits=qualified_commits, | ||
generator_version=generator_version, | ||
is_monorepo=is_monorepo, | ||
) | ||
|
||
|
||
def __filter_qualified_commit(paths: Dict[str, str], commit: Commit) -> (Commit, str): | ||
""" | ||
Returns a tuple of a commit and libray_name. | ||
A qualified commit means at least one file changes in that commit is | ||
within the versioned proto_path in paths. | ||
|
||
:param paths: a mapping from versioned proto_path to library_name. | ||
:param commit: a commit under consideration. | ||
:return: a tuple of a commit and library_name if the commit is | ||
qualified; otherwise an empty tuple. | ||
""" | ||
for file in commit.stats.files.keys(): | ||
versioned_proto_path = find_versioned_proto_path(file) | ||
if versioned_proto_path in paths: | ||
return commit, paths[versioned_proto_path] | ||
return () | ||
|
||
|
||
def __combine_commit_messages( | ||
latest_commit: str, | ||
baseline_commit: str, | ||
commits: Dict[Commit, str], | ||
generator_version: str, | ||
is_monorepo: bool, | ||
) -> str: | ||
messages = [ | ||
f"This pull request is generated with proto changes between googleapis commit {baseline_commit} (exclusive) and {latest_commit} (inclusive).", | ||
"Qualified commits are:", | ||
] | ||
for commit in commits: | ||
short_sha = commit.hexsha[:7] | ||
messages.append( | ||
f"[googleapis/googleapis@{short_sha}](https://github.com/googleapis/googleapis/commit/{commit.hexsha})" | ||
) | ||
|
||
for commit, library_name in commits.items(): | ||
convention, _, summary = commit.message.partition(":") | ||
formatted_message = ( | ||
f"{convention}: [{library_name}]{str(summary).rstrip()}" | ||
if is_monorepo | ||
else f"{convention}:{str(summary).rstrip()}" | ||
) | ||
messages.append(__wrap_nested_commit(formatted_message)) | ||
messages.append( | ||
__wrap_nested_commit( | ||
f"feat: Regenerate with the Java code generator (gapic-generator-java) v{generator_version}" | ||
) | ||
) | ||
|
||
return "\n".join(messages) | ||
|
||
|
||
def __wrap_nested_commit(message: str) -> str: | ||
result = ["BEGIN_NESTED_COMMIT", message, "END_NESTED_COMMIT"] | ||
return "\n".join(result) | ||
|
||
|
||
if __name__ == "__main__": | ||
main() |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -17,12 +17,14 @@ | |
import unittest | ||
from distutils.dir_util import copy_tree | ||
from distutils.file_util import copy_file | ||
from filecmp import cmp | ||
from filecmp import dircmp | ||
|
||
from git import Repo | ||
from pathlib import Path | ||
from typing import List | ||
from typing import Dict | ||
|
||
from library_generation.generate_pr_description import generate_pr_descriptions | ||
from library_generation.generate_repo import generate_from_yaml | ||
from library_generation.model.generation_config import from_yaml, GenerationConfig | ||
from library_generation.test.compare_poms import compare_xml | ||
|
@@ -49,6 +51,35 @@ | |
|
||
|
||
class IntegrationTest(unittest.TestCase): | ||
def test_get_commit_message_success(self): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I added this as an integration test because it required network connection for git clone and checkout of googleapis repository. |
||
repo_url = "https://github.com/googleapis/googleapis.git" | ||
config_files = self.__get_config_files(config_dir) | ||
monorepo_baseline_commit = "a17d4caf184b050d50cacf2b0d579ce72c31ce74" | ||
split_repo_baseline_commit = "679060c64136e85b52838f53cfe612ce51e60d1d" | ||
for repo, config_file in config_files: | ||
baseline_commit = ( | ||
monorepo_baseline_commit | ||
if repo == "google-cloud-java" | ||
else split_repo_baseline_commit | ||
) | ||
description = generate_pr_descriptions( | ||
generation_config_yaml=config_file, | ||
repo_url=repo_url, | ||
baseline_commit=baseline_commit, | ||
) | ||
description_file = f"{config_dir}/{repo}/pr-description.txt" | ||
if os.path.isfile(f"{description_file}"): | ||
os.remove(f"{description_file}") | ||
with open(f"{description_file}", "w+") as f: | ||
f.write(description) | ||
self.assertTrue( | ||
cmp( | ||
f"{config_dir}/{repo}/pr-description-golden.txt", | ||
f"{description_file}", | ||
) | ||
) | ||
os.remove(f"{description_file}") | ||
|
||
def test_generate_repo(self): | ||
shutil.rmtree(f"{golden_dir}", ignore_errors=True) | ||
os.makedirs(f"{golden_dir}", exist_ok=True) | ||
|
@@ -150,7 +181,7 @@ def __pull_repo_to(cls, default_dest: Path, repo: str, committish: str) -> str: | |
repo = Repo(dest) | ||
else: | ||
dest = default_dest | ||
repo_dest = f"{golden_dir}/{repo}" | ||
shutil.rmtree(dest, ignore_errors=True) | ||
repo_url = f"{repo_prefix}/{repo}" | ||
print(f"Cloning repository {repo_url}") | ||
repo = Repo.clone_from(repo_url, dest) | ||
|
@@ -169,6 +200,8 @@ def __get_library_names_from_config(cls, config: GenerationConfig) -> List[str]: | |
def __get_config_files(cls, path: str) -> List[tuple[str, str]]: | ||
config_files = [] | ||
for sub_dir in Path(path).resolve().iterdir(): | ||
if sub_dir.is_file(): | ||
continue | ||
repo = sub_dir.name | ||
if repo == "golden": | ||
continue | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please capture the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @chingor13 Do you mind sharing how There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think it's in go/client-user-guide#tracking-library-packages There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I can see the value of including them in googleapis commit messages, but since they are not propagated to release notes of google-cloud-java anyway, I'm not sure how we can track their release statuses. In addition, do we need to track the release statuses of a certain There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The metrics tracker links the release PRs with the source PRs either via the commit SHA or the PR# reference |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,17 @@ | ||
This pull request is generated with proto changes between googleapis commit a17d4caf184b050d50cacf2b0d579ce72c31ce74 (exclusive) and 1a45bf7393b52407188c82e63101db7dc9c72026 (inclusive). | ||
Qualified commits are: | ||
[googleapis/googleapis@7a9a855](https://github.com/googleapis/googleapis/commit/7a9a855287b5042410c93e5a510f40efd4ce6cb1) | ||
[googleapis/googleapis@c7fd8bd](https://github.com/googleapis/googleapis/commit/c7fd8bd652ac690ca84f485014f70b52eef7cb9e) | ||
BEGIN_NESTED_COMMIT | ||
feat: [document-ai] expose model_type in v1 processor, so that user can see the model_type after get or list processor version | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we add a test case for a commit that contains multiple conventional commit messages? Like this one? I think the current logic that adds library name may not take this scenario into consideration. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done. |
||
|
||
PiperOrigin-RevId: 603727585 | ||
END_NESTED_COMMIT | ||
BEGIN_NESTED_COMMIT | ||
feat: [document-ai] add model_type in v1beta3 processor proto | ||
|
||
PiperOrigin-RevId: 603726122 | ||
END_NESTED_COMMIT | ||
BEGIN_NESTED_COMMIT | ||
feat: Regenerate with the Java code generator (gapic-generator-java) v2.34.0 | ||
END_NESTED_COMMIT |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,17 @@ | ||
This pull request is generated with proto changes between googleapis commit 679060c64136e85b52838f53cfe612ce51e60d1d (exclusive) and fc3043ebe12fb6bc1729c175e1526c859ce751d8 (inclusive). | ||
Qualified commits are: | ||
[googleapis/googleapis@fbcfef0](https://github.com/googleapis/googleapis/commit/fbcfef09510b842774530989889ed1584a8b5acb) | ||
[googleapis/googleapis@63d2a60](https://github.com/googleapis/googleapis/commit/63d2a60056ad5b156c05c7fb13138fc886c3b739) | ||
BEGIN_NESTED_COMMIT | ||
fix: extend timeouts for deleting snapshots, backups and tables | ||
|
||
PiperOrigin-RevId: 605388988 | ||
END_NESTED_COMMIT | ||
BEGIN_NESTED_COMMIT | ||
chore: update retry settings for backup rpcs | ||
|
||
PiperOrigin-RevId: 605367937 | ||
END_NESTED_COMMIT | ||
BEGIN_NESTED_COMMIT | ||
feat: Regenerate with the Java code generator (gapic-generator-java) v2.35.0 | ||
END_NESTED_COMMIT |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need to consider a scenario that the commit message may include multiple conventional commits.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done, verified in unit test.