Skip to content
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

Fix release merge script #4273

Merged
merged 2 commits into from
May 23, 2023
Merged

Conversation

frangio
Copy link
Contributor

@frangio frangio commented May 23, 2023

In the case of conflicts when merging release-vX.Y with master, we are not doing our best to resolve them automatically.

This PR:

  1. Adds || true to allow git merge to continue even with conflicts. They should be resolved by the following rm and git add commands. If they are not resolved by that, git commit will fail, we have to resolve manually.
  2. Adds --diff-filter=D to git diff, this just makes sure we are only listing deleted files and not anything else that has changed.
  3. Uses git ls-files --deleted to make sure we don't git add anything that we don't intend to.

2 and 3 are not strictly related to the original issue but are just making the script more robust.

@frangio frangio requested a review from ernestognw May 23, 2023 19:19
@changeset-bot
Copy link

changeset-bot bot commented May 23, 2023

⚠️ No Changeset found

Latest commit: 95ed609

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

Copy link
Member

@ernestognw ernestognw left a comment

Choose a reason for hiding this comment

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

LGTM, I'd just add some more comments since these are the sort of things we forget after a while

ernestognw
ernestognw previously approved these changes May 23, 2023
Copy link
Member

@ernestognw ernestognw left a comment

Choose a reason for hiding this comment

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

LGTM, I'd just add some more comments since these are the sort of things we forget after a while

@frangio frangio merged commit 7e814a3 into OpenZeppelin:master May 23, 2023
@frangio frangio deleted the fix-merge-script branch May 23, 2023 21:21
frangio added a commit that referenced this pull request May 23, 2023
Co-authored-by: Ernesto García <[email protected]>
(cherry picked from commit 7e814a3)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants