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: compare BASE with merged state instead of HEAD directly #13

Merged
merged 2 commits into from
Mar 12, 2025

Conversation

swade1987
Copy link
Owner

@swade1987 swade1987 commented Mar 11, 2025

This PR addresses issue #11, where the action incorrectly indicated that changes to the BASE branch would be reverted when the PR is merged.

Problem

The previous implementation compared the kustomize output from the BASE branch directly with the HEAD branch. This approach fails to account for git's merge behaviour and can lead to misleading diffs:

Old Base -> BASE
         \
           -> HEAD

When someone creates a feature branch without first pulling the latest changes, parallel changes made to BASE would appear as if they would be reverted by the PR, even though git's actual merge behavior would preserve them.

Solution

Instead of comparing BASE with HEAD directly, this PR:

  1. Builds the kustomize output for the BASE branch
  2. Creates a temporary merge branch from BASE
  3. Merges HEAD into this temporary branch (simulating what git would do during PR merge)
  4. It builds the kustomize output for the merged state
  5. Compares the BASE output with the merged output

This approach shows only the changes introduced by merging the PR, providing a more accurate representation of the final state.

Additional Improvements

  1. Added proper merge conflict detection and handling
  2. Improved cleanup of temporary git branches
  3. Enhanced debug logging for the merge process
  4. Maintained backward compatibility with existing input parameters

Closes #11

How has this been tested?

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I've read the CONTRIBUTION guide
  • I have signed-off my commits with git commit -s

Copy link

@joshgubler joshgubler left a comment

Choose a reason for hiding this comment

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

This looks great! Thank you!

@swade1987 swade1987 marked this pull request as ready for review March 12, 2025 15:33
@swade1987 swade1987 merged commit a2444f0 into next Mar 12, 2025
3 checks passed
@swade1987 swade1987 deleted the merge-branch branch March 12, 2025 15:34
@swade1987
Copy link
Owner Author

🎉 This PR is included in version 0.3.0-next.1 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

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