Skip to content

Rename --repo argument to --selected-repo to avoid confusion in the resolver workflow #7287

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

Merged
merged 7 commits into from
Mar 20, 2025

Conversation

tawago
Copy link
Contributor

@tawago tawago commented Mar 16, 2025

  • This change is worth documenting at https://docs.all-hands.dev/
  • Include this change in the Release Notes. If checked, you must provide an end-user friendly description for your change below

End-user friendly description of the problem this fixes or functionality that this introduces.
The command-line argument --repo was ambiguous, leading to token validation failures when running the resolver. By renaming it to --selected-repo, the expected format is now more explicit and less prone to errors.


Give a summary of what the PR does, explaining any non-trivial design decisions.
Follow up of #7065

In #7065, the bug inside identify_token function was resolved by adding repo argument and associated validation; however, the main function of resolve_issue.py was passing the new argument as a incorrect string format to that function.

Within the file resolve_issue.py, repo can mean both {owner}/{repo} format string and single {repo} name. To avoid the confusion and to fix the bug properly, this PR tries to rename the argument.


Link of any specific issues this addresses.

@tawago tawago changed the title Rename --repo argument to --owner-repo to avoid confusion in resolver Rename --repo argument to --owner-repo to avoid confusion in the resolver workflow Mar 16, 2025
@tawago tawago changed the title Rename --repo argument to --owner-repo to avoid confusion in the resolver workflow Rename --repo argument to --selected-repo to avoid confusion in the resolver workflow Mar 17, 2025
@tawago
Copy link
Contributor Author

tawago commented Mar 17, 2025

I kinda wanna add some e2e tests to make sure that the resolver workflow always works in all supported environments (github_token, PAT, gitlab token) but probably not in this PR's scope.

@enyst enyst requested a review from malhotra5 March 17, 2025 18:29
@enyst
Copy link
Collaborator

enyst commented Mar 17, 2025

Looks good to me, thank you! I'd love it if @malhotra5 could take a quick look too.

@malhotra5
Copy link
Contributor

This LGTM

The workflow file, however, will need to be updated on a future PR

The current changes to the resolver won't be available until a release is made (which is when the workflow file should get updated)

We should be using release SHA to manage this, but that hasn't been implemented yet

@tawago
Copy link
Contributor Author

tawago commented Mar 18, 2025

@malhotra5

Just to confirm:
Do you want me to create a separate PR for the workflow diff only? or This PR should get merged right before 0.29.0 release?

@malhotra5
Copy link
Contributor

Do you want me to create a separate PR for the workflow diff only

Yes that would be greatly appreciated!

We'll merge this PR right away. The separate PR you create for the workflow diff will get merged right after the next release.

Copy link
Contributor

@malhotra5 malhotra5 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@malhotra5 malhotra5 enabled auto-merge (squash) March 20, 2025 04:42
@malhotra5 malhotra5 merged commit 3e3b2aa into All-Hands-AI:main Mar 20, 2025
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants