Skip to content

Commit 815d605

Browse files
Fix: Remove deprecated nested argument group
- Removes the deprecated nested `add_argument_group` for owner/repo. - `--owner` and `--repo` are now top-level arguments. - Implements manual validation logic after parsing to ensure correct argument pairing and exclusivity with `--url`. - This addresses the DeprecationWarning while maintaining the intended argument behavior.
1 parent 29a3223 commit 815d605

File tree

1 file changed

+38
-26
lines changed

1 file changed

+38
-26
lines changed

scripts/gha/get_pr_review_comments_standalone.py

Lines changed: 38 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -150,26 +150,24 @@ def parse_repo_url(url_string):
150150
required=True,
151151
help="Pull request number."
152152
)
153-
repo_spec_group = parser.add_mutually_exclusive_group()
154-
repo_spec_group.add_argument(
153+
# Arguments for repository specification
154+
parser.add_argument(
155155
"--url",
156156
type=str,
157157
default=None,
158-
help="Full GitHub repository URL (e.g., https://github.com/owner/repo or [email protected]:owner/repo.git). Overrides --owner/--repo and git detection."
158+
help="Full GitHub repository URL (e.g., https://github.com/owner/repo or [email protected]:owner/repo.git). Takes precedence over --owner/--repo."
159159
)
160-
# Create a sub-group for owner/repo pair if not using URL
161-
owner_repo_group = repo_spec_group.add_argument_group('owner_repo_pair', 'Specify owner and repository name (used if --url is not provided)')
162-
owner_repo_group.add_argument(
160+
parser.add_argument(
163161
"--owner",
164162
type=str,
165-
default=determined_owner,
166-
help=f"Repository owner. {'Default: ' + determined_owner if determined_owner else 'Required if --url is not used and not determinable from git.'}"
163+
default=determined_owner, # Default to auto-detected
164+
help=f"Repository owner. Used if --url is not provided. {'Default: ' + determined_owner if determined_owner else 'Required if --url is not used and not determinable from git.'}"
167165
)
168-
owner_repo_group.add_argument(
166+
parser.add_argument(
169167
"--repo",
170168
type=str,
171-
default=determined_repo,
172-
help=f"Repository name. {'Default: ' + determined_repo if determined_repo else 'Required if --url is not used and not determinable from git.'}"
169+
default=determined_repo, # Default to auto-detected
170+
help=f"Repository name. Used if --url is not provided. {'Default: ' + determined_repo if determined_repo else 'Required if --url is not used and not determinable from git.'}"
173171
)
174172
parser.add_argument(
175173
"--token",
@@ -211,7 +209,16 @@ def parse_repo_url(url_string):
211209
final_owner = None
212210
final_repo = None
213211

212+
# Logic for determining owner and repo:
213+
# 1. If --url is provided, use it. Error if --owner or --repo are also explicitly set.
214+
# 2. Else if --owner and --repo are provided, use them.
215+
# 3. Else (neither --url nor --owner/--repo pair explicitly set), use auto-detected values (which are defaults for args.owner/args.repo).
216+
214217
if args.url:
218+
if args.owner != determined_owner or args.repo != determined_repo: # Check if owner/repo were explicitly changed from defaults
219+
sys.stderr.write("Error: Cannot use --owner/--repo when --url is specified.\n")
220+
parser.print_help()
221+
sys.exit(1)
215222
parsed_owner, parsed_repo = parse_repo_url(args.url)
216223
if parsed_owner and parsed_repo:
217224
final_owner = parsed_owner
@@ -221,25 +228,30 @@ def parse_repo_url(url_string):
221228
sys.stderr.write(f"Error: Invalid URL format provided: {args.url}. Expected https://github.com/owner/repo or [email protected]:owner/repo.git\n")
222229
parser.print_help()
223230
sys.exit(1)
224-
# If URL is not provided, check owner/repo. They default to determined_owner/repo.
225-
elif args.owner and args.repo:
231+
elif (args.owner != determined_owner or args.repo != determined_repo) or (not determined_owner and args.owner and args.repo):
232+
# This condition means:
233+
# 1. User explicitly set --owner or --repo to something different than auto-detected OR
234+
# 2. Auto-detection failed (determined_owner is None) AND user provided both owner and repo.
235+
if not args.owner or not args.repo:
236+
sys.stderr.write("Error: Both --owner and --repo must be specified if one is provided (and --url is not used).\n")
237+
parser.print_help()
238+
sys.exit(1)
226239
final_owner = args.owner
227240
final_repo = args.repo
228-
# If these values are different from the auto-detected ones (i.e., user explicitly provided them),
229-
# or if auto-detection failed and these are the only source.
230-
if (args.owner != determined_owner or args.repo != determined_repo) and (determined_owner or determined_repo):
231-
sys.stderr.write(f"Using repository from --owner/--repo args: {final_owner}/{final_repo}\n")
232-
# If auto-detection worked and user didn't override, the initial "Determined repository..." message is sufficient.
233-
elif args.owner or args.repo: # Only one of owner/repo was specified (and not --url)
234-
sys.stderr.write("Error: Both --owner and --repo must be specified if one is provided and --url is not used.\n")
235-
parser.print_help()
236-
sys.exit(1)
237-
# If --url, --owner, --repo are all None, it means auto-detection failed AND user provided nothing.
238-
# This case is caught by the final check below.
239-
241+
sys.stderr.write(f"Using repository from --owner/--repo args: {final_owner}/{final_repo}\n")
242+
elif args.owner and args.repo: # Using auto-detected values which are now in args.owner and args.repo
243+
final_owner = args.owner
244+
final_repo = args.repo
245+
# The "Determined repository..." message was already printed if successful.
246+
else: # Handles cases like only one of owner/repo being set after defaults, or auto-detection failed and nothing/partial was given.
247+
if (args.owner and not args.repo) or (not args.owner and args.repo):
248+
sys.stderr.write("Error: Both --owner and --repo must be specified if one is provided (and --url is not used).\n")
249+
parser.print_help()
250+
sys.exit(1)
251+
# If it reaches here and final_owner/repo are still None, it means auto-detection failed and user didn't provide valid pair.
240252

241253
if not final_owner or not final_repo:
242-
sys.stderr.write("Error: Could not determine repository. Please specify --url, or both --owner and --repo, or ensure git remote 'origin' is configured correctly.\n")
254+
sys.stderr.write("Error: Could not determine repository. Please specify --url, OR both --owner and --repo, OR ensure git remote 'origin' is configured correctly.\n")
243255
parser.print_help()
244256
sys.exit(1)
245257

0 commit comments

Comments
 (0)