Copilot commented on code in PR #14196: URL: https://github.com/apache/cloudstack/pull/14196#discussion_r4047629525
########## .github/scripts/check_since_annotations.py: ########## @@ -0,0 +1,224 @@ +#!/usr/bin/env python3 +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you 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. +# +# Checks @APICommand/@Parameter/@Param `since` usage on branches that have +# moved past the old 4.x version scheme (i.e. project version >= 24): +# +# 1. A newly added since="4.x" is flagged - contributors often add this out +# of muscle memory even though the project is now versioned e.g. 24.0.0. +# 2. A brand-new @APICommand/@Parameter/@Param (its annotation AND the +# class/field it annotates are both newly added together) that has no +# since attribute at all is flagged - new API surface should record when +# it was introduced. Editing an existing annotation (its declaration +# line is not part of the diff) never requires since, even if the +# existing element never had one. +# +# Only lines actually added by the PR are inspected. A value/field is +# ignored if the same PR also removes the identical since="..." value, or +# the identical field/class name, elsewhere in the same file's diff - this +# covers a field being moved or reformatted rather than a genuinely new +# API/param/response field. + +import argparse +import re +import subprocess +import sys + +ANNOTATION_START_RE = re.compile(r"@(Param|Parameter|APICommand)\s*\(") +SINCE_RE = re.compile(r'since\s*=\s*"(4\.\d[\w.]*)"') +HAS_SINCE_RE = re.compile(r"\bsince\s*=") +VERSION_RE = re.compile(r"<artifactId>cloudstack</artifactId>\s*<version>([^<]+)</version>") +FIELD_DECL_RE = re.compile(r"^\s*(?:private|protected|public)\b[^=;(){}]*?(\w+)\s*;\s*$") +CLASS_DECL_RE = re.compile(r"^\s*(?:public\s+)?(?:final\s+)?class\s+(\w+)") Review Comment: FIELD_DECL_RE does not match the common CloudStack pattern of `@Parameter-annotated` fields with default initializers (e.g. "private boolean listAll = false;"). This means brand-new parameters/fields added with an initializer will be treated as "not new" and won't be flagged for missing since. ########## .github/scripts/check_since_annotations.py: ########## @@ -0,0 +1,224 @@ +#!/usr/bin/env python3 +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you 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. +# +# Checks @APICommand/@Parameter/@Param `since` usage on branches that have +# moved past the old 4.x version scheme (i.e. project version >= 24): +# +# 1. A newly added since="4.x" is flagged - contributors often add this out +# of muscle memory even though the project is now versioned e.g. 24.0.0. +# 2. A brand-new @APICommand/@Parameter/@Param (its annotation AND the +# class/field it annotates are both newly added together) that has no +# since attribute at all is flagged - new API surface should record when +# it was introduced. Editing an existing annotation (its declaration +# line is not part of the diff) never requires since, even if the +# existing element never had one. +# +# Only lines actually added by the PR are inspected. A value/field is +# ignored if the same PR also removes the identical since="..." value, or +# the identical field/class name, elsewhere in the same file's diff - this +# covers a field being moved or reformatted rather than a genuinely new +# API/param/response field. + +import argparse +import re +import subprocess +import sys + +ANNOTATION_START_RE = re.compile(r"@(Param|Parameter|APICommand)\s*\(") +SINCE_RE = re.compile(r'since\s*=\s*"(4\.\d[\w.]*)"') +HAS_SINCE_RE = re.compile(r"\bsince\s*=") +VERSION_RE = re.compile(r"<artifactId>cloudstack</artifactId>\s*<version>([^<]+)</version>") +FIELD_DECL_RE = re.compile(r"^\s*(?:private|protected|public)\b[^=;(){}]*?(\w+)\s*;\s*$") +CLASS_DECL_RE = re.compile(r"^\s*(?:public\s+)?(?:final\s+)?class\s+(\w+)") + + +def read_project_version(pom_path: str) -> str: + with open(pom_path, encoding="utf-8") as f: + content = f.read() + match = VERSION_RE.search(content) + if not match: + raise SystemExit(f"Could not find the cloudstack project version in {pom_path}") + return match.group(1) + + +def major_version(version: str) -> int: + match = re.match(r"(\d+)", version) + if not match: + raise SystemExit(f"Could not parse a major version from '{version}'") + return int(match.group(1)) + + +def git_diff(base: str, head: str) -> str: + return subprocess.run( + ["git", "diff", "--no-color", "--unified=0", base, head, "--", "*.java"], + check=True, + capture_output=True, + text=True, + ).stdout Review Comment: The git diff pathspec "*.java" only matches Java files in the repository root, so this check will miss changes to Java sources in subdirectories (which is effectively all of CloudStack). Use a recursive pathspec so newly added annotations in any module are inspected. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
