Conversation
…dme with directions.
|
| Objecting: Update reusable workflow references in multiple git repositories like a massive find and replace | ||
| 1. Traverses all git repositories under a parent directory. | ||
| 2. Searches for .github/workflows/*.yml and *.yaml files. | ||
| 3. Find and replace lines within the workfloew. |
| print(f"Error processing repository {repo}: {e}") | ||
| break # Remove this break to process all repositories | ||
|
|
||
| reponse = input( |
| changes = [] | ||
| for workflow_file in find_workflow_files(repo): | ||
| text = workflow_file.read_text(encoding="utf-8") | ||
| matches = list(re.compile(args.find).finditer(text)) |
There was a problem hiding this comment.
You can define a variable with the pattern re.compile(args.find) at the beginning of this function, instead of have it twice line 97 and line 105
| parser.add_argument( | ||
| "--replace", help="String to replace in workflow references", required=True) | ||
|
|
||
| args = parser.parse_args() |
There was a problem hiding this comment.
You use args as a global object, so all your functions have access to the args object. I recommend creating variables for the relevant parameters and defining arguments in your functions. e.g. find=args.find and collect_changes(repo, find). This way, you decouple your functions from the CLI parser.
| repo, | ||
| ) | ||
| print(f"\nDifferences in repository {repo}:\n{diff}\n") | ||
| return True |
There was a problem hiding this comment.
This function always returns True, but since you don't use its result (line 319), you could remove this return statement.
| print(f"Modified files in {repo}:") | ||
| for file in modified_files: | ||
| print(f"{file}") | ||
| # commit_and_push(repo) |
| print("\nScanning repositories for proposed changes...") | ||
| print("--------------------------------------------------\n") | ||
|
|
||
| for repo in repos: |
There was a problem hiding this comment.
@kron-spar I recommend you split the logic into the main function into different stages: 1) collect_changes, 2) apply_changes. Right now,
Step 1: collect_changes, display, and ask user - This step won't update anything
Step 2: For each repository, apply the changes, then commit & push, and if something went wrong, you need to clean up.
Something like this
repos = find_repositories(args.root_dir)
# Step 1: collect the changes
for repo in repos:
changes = collect_changes(repo)
if changes:
proposed_changes[repo] = changes
print(f"\nFound changes in {repo}")
for change in changes:
print(f"File: {change['file']}")
print(f" Before: {change['before']}")
print(f" After: {change['after']}")
if not proposed_changes:
print("\nNo proposed changes found.")
sys.exit(0)
response = input("\nDo you want to apply these changes? (y/n): ")
if response.lower() != 'y':
print("Aborting.")
sys.exit(0)
# Step 2: apply to all repos
for repo in proposed_changes:
try:
prepare_repository(repo)
modified_files = apply_changes(repo)
commit_and_push(repo)
except RuntimeError as e:
print(f"Error processing repository {repo}: {e}")
cleanup_repository(repo) # With this restructure flow, you only need to apply cleanup if something went wrong
aelkiss
left a comment
There was a problem hiding this comment.
At first glance the logic looks correct. I agree with Lianet's suggested changes.
Since this is general enough that it could be something we re-use in the future, it's probably worth thinking about some testing here. The challenge is that in that case we don't want to actually git commit, etc. The way to deal with that is dependency injection -- we need a class/object that we can provide that can either do the real git operations or can serve as a mock where we can verify that specific functions have been called, but without actually doing the git operations.
I'd suggest making the changes Lianet suggests, then looking at what a basic integration test might look like, and we can look at how to implement that. Let me know what you think.
|
After discussing with @kron-spar I think we can wait on automated tests for now since we expect this to be a one-time thing. If we do end up needing to use this again, then I think it's worth thinking more about automated tests at that point. |
|
|
The python script is designed to adjust all workflow repos and updated the Readme with directions. It takes a directory(s), a given branch name, and a string to
findand a string toreplaceit.