-
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 9 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,86 @@ | ||
#!/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 click | ||
|
||
from library_generation.model.generation_config import from_yaml | ||
from library_generation.utilities import get_commit_messages | ||
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. | ||
""", | ||
) | ||
@click.option( | ||
"--baseline-commit", | ||
required=True, | ||
type=str, | ||
help=""" | ||
The baseline commit 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( | ||
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 another commit for the generator version update? This is currently done as 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. Generator updates will not use GitHub action as it only happens once in two weeks. I'll setup a meeting to discuss this issue. 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. Added in the PR description. |
||
repo_url=repo_url, | ||
latest_commit=config.googleapis_commitish, | ||
baseline_commit=baseline_commit, | ||
paths=paths, | ||
) | ||
|
||
|
||
if __name__ == "__main__": | ||
main() |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -23,6 +23,8 @@ | |
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 | ||
from library_generation.test.compare_poms import compare_xml | ||
|
@@ -45,6 +47,24 @@ | |
|
||
|
||
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" | ||
baseline_commit = "a17d4caf184b050d50cacf2b0d579ce72c31ce74" | ||
description = generate_pr_descriptions( | ||
f"{config_dir}/google-cloud-java/{config_name}", | ||
JoeWang1127 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
repo_url=repo_url, | ||
baseline_commit=baseline_commit, | ||
) | ||
# There are five commits from googleapis commitish to baseline commit, | ||
# inclusively. Only two of the commits contain changes in proto_path | ||
# that are in configuration. Therefore, only two commit messages should | ||
# be included in the PR description. | ||
self.assertFalse("a17d4caf184b050d50cacf2b0d579ce72c31ce74" in description) | ||
JoeWang1127 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
self.assertFalse("9063da8b4d45339db4e2d7d92a27c6708620e694" in description) | ||
self.assertTrue("7a9a855287b5042410c93e5a510f40efd4ce6cb1" in description) | ||
self.assertTrue("c7fd8bd652ac690ca84f485014f70b52eef7cb9e" in description) | ||
self.assertFalse("a17d4caf184b050d50cacf2b0d579ce72c31ce74" in description) | ||
|
||
def test_generate_repo(self): | ||
shutil.rmtree(f"{golden_dir}", ignore_errors=True) | ||
os.makedirs(f"{golden_dir}", exist_ok=True) | ||
|
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.
Is baseline-commit inclusive?
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.
No.
The baseline commit is included in the previous generation.
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.
Can you please add comments to make it clear?
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.
Changed the comments.