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: deletionProposed branch throws error for main revisions #4056

Merged
merged 3 commits into from
Oct 11, 2023

Conversation

natasha41575
Copy link
Contributor

@natasha41575 natasha41575 commented Oct 10, 2023

When following these steps:

  1. Create two PackageRevisions belonging to different packages in the same repo and propose and publish both.
  2. Propose deletion of both "main" packagerevisions that get created.
  3. Delete the first PackageRevision from "main" and wait over a minute until the sync has happened.
  4. Delete the second PackageRevision from "main" and an error occurs complaining that the commit is incorrect.

The error is thrown because when we try to delete the deletionProposed branch, we are passing in the commit from the latest ref on the main branch. In our case, when we try to delete the second packagerevision, we pass in the latest commit from main which was created when the first packagerevision was deleted. This commit doesn't exist in the second deletionProposed branch because the deletion of the first packagerevision happened on the main branch after the second packagerevision was proposed for deletion.

Since we're trying to delete the deletionProposed branch entirely and are using its existence as a boolean, I don't think we care what commit its on. We can pass in the zero-hash commit for it to delete the branch regardless of what commit its on. This makes the error go away for me locally.

@natasha41575 natasha41575 changed the title pass in zero-hash instead of current commit when deleting deletionPro… fix: deletionProposed branch throws error for main revisions Oct 10, 2023
@johnbelamaric
Copy link
Contributor

Makes sense to me. Since we are deleting, the specific commit is not critical.

@natasha41575 natasha41575 force-pushed the deletionFromMainIssue branch 3 times, most recently from 5ec1782 to 9bb0668 Compare October 11, 2023 18:33
…posed branch

Signed-off-by: natasha41575 <natashasarkar@google.com>
Signed-off-by: natasha41575 <natashasarkar@google.com>
Signed-off-by: natasha41575 <natashasarkar@google.com>
@natasha41575 natasha41575 marked this pull request as ready for review October 11, 2023 20:17
@natasha41575 natasha41575 merged commit 34cf103 into kptdev:main Oct 11, 2023
11 checks passed
@natasha41575 natasha41575 deleted the deletionFromMainIssue branch October 11, 2023 21:38
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.

None yet

3 participants