Skip to content

Conversation

@cottsay
Copy link
Member

@cottsay cottsay commented Apr 21, 2025

If something goes wrong, use dry run pushes to ensure we don't destroy any data in the repository.

  1. Ensure we're not overwriting tags
  2. Push upstream branch. This is the ONLY branch we should ever force-push, and is an intermediate staging branch anyway.
  3. Ensure we're not overwriting any branches
  4. Push the branches and tags

!!! I haven't actually tested this yet

@cottsay cottsay requested a review from nuclearsandwich April 21, 2025 20:41
@cottsay cottsay self-assigned this Apr 21, 2025
subprocess.check_call(['git', 'push', 'origin', '--tags', '--dry-run'])
subprocess.check_call(['git', 'push', 'origin', 'upstream', '--force'])
subprocess.check_call(['git', 'push', 'origin', '--all', '--dry-run'])
subprocess.check_call(['git', 'push', 'origin', '--tags', '--all'])
Copy link
Member

@nuclearsandwich nuclearsandwich Apr 21, 2025

Choose a reason for hiding this comment

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

This needs to be split since --all and --tags can't both be used. There's a synonym --branches for --all that may be easier to read.

# 3. Ensure we're not overwriting any branches
# 4. Push the branches and tags
subprocess.check_call(['git', 'push', 'origin', '--tags', '--dry-run'])
subprocess.check_call(['git', 'push', 'origin', 'upstream', '--force'])
Copy link
Member

Choose a reason for hiding this comment

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

There are situations where upstream/* tags may also technically be changed between releases which would result in an inconsistent state between the current upstream state and the previous tag but the only real-world examples I know of were in the early ROS 2 days with fastrtps and thankfully it doesn't happen any more.

Copy link
Member

Choose a reason for hiding this comment

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

I don't think this requires action right away. It's a hint in case this change creates additional unexpected failures pushing other upstream tags.

Copy link
Member

@nuclearsandwich nuclearsandwich left a comment

Choose a reason for hiding this comment

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

Left a suggestion which I'm pretty sure is a required change and clarified that the other comment is an observation rather than something actionable.

subprocess.check_call(['git', 'push', 'origin', '--tags', '--dry-run'])
subprocess.check_call(['git', 'push', 'origin', 'upstream', '--force'])
subprocess.check_call(['git', 'push', 'origin', '--all', '--dry-run'])
subprocess.check_call(['git', 'push', 'origin', '--tags', '--all'])
Copy link
Member

Choose a reason for hiding this comment

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

Putting my observation into suggestion.

Suggested change
subprocess.check_call(['git', 'push', 'origin', '--tags', '--all'])
subprocess.check_call(['git', 'push', 'origin', '--tags'])
subprocess.check_call(['git', 'push', 'origin', '--branches'])

@cottsay cottsay force-pushed the cottsay/migration-dry-runs branch from cc70677 to c8052d9 Compare April 23, 2025 19:15
@emersonknapp emersonknapp added the persistent Issue/PR won't be marked as stale if inactive for a while. label May 7, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

persistent Issue/PR won't be marked as stale if inactive for a while.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants