jedcunningham commented on code in PR #31024:
URL: https://github.com/apache/airflow/pull/31024#discussion_r1186184793


##########
dev/breeze/src/airflow_breeze/commands/release_candidate_command.py:
##########
@@ -164,9 +164,9 @@ def push_artifacts_to_asf_repo(version, repo_root):
             check=True,
         )
         console_print("Files pushed to svn")
+        os.chdir(repo_root)
         # Remove old releases
         remove_old_releases(version, repo_root)
-        os.chdir(repo_root)
         run_command(["rm", "-rf", "asf-dist"], dry_run_override=DRY_RUN, 
check=True)

Review Comment:
   I'm just seeing #30944, and I feel like instead of moving where we remove 
old releases, we shouldn't do this cleanup in the push method. My 2c.



##########
dev/breeze/src/airflow_breeze/commands/release_candidate_command.py:
##########
@@ -164,9 +164,9 @@ def push_artifacts_to_asf_repo(version, repo_root):
             check=True,
         )
         console_print("Files pushed to svn")
+        os.chdir(repo_root)

Review Comment:
   I see. Can we the change `remove_old_releases` to do this "reset to repo 
root" instead?



##########
dev/breeze/src/airflow_breeze/commands/release_candidate_command.py:
##########
@@ -164,9 +164,9 @@ def push_artifacts_to_asf_repo(version, repo_root):
             check=True,
         )
         console_print("Files pushed to svn")
+        os.chdir(repo_root)
         # Remove old releases
         remove_old_releases(version, repo_root)
-        os.chdir(repo_root)
         run_command(["rm", "-rf", "asf-dist"], dry_run_override=DRY_RUN, 
check=True)

Review Comment:
   (That pattern is why my testing failed when I added the old release removal 
actually, I had no clue we'd already deleted the svn repo.)



-- 
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]

Reply via email to