Revert on-path-change on pr merge in bitbucket#2247
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Summary of Changes
Hello @zakisk, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
This pull request is a direct revert of a previous commit that introduced a specific handling mechanism for merge commits in Bitbucket Data Center push events, intended to fix an on-path-change issue. By reverting this change, the complex logic for identifying the 'second parent commit SHA' and its corresponding test cases are removed, effectively undoing the prior attempt to resolve the problem and returning to the previous state of payload parsing for Bitbucket Data Center.
Highlights
- Revert Previous Change: This pull request fully reverts commit
f9fd3f3f7ba68db845ec2a0b723798ed812f4a1e, which aimed to address an issue withon-path-changeon PR merge events in Bitbucket Data Center. - Bitbucket Data Center Payload Parsing: The logic that attempted to identify and use the 'second parent commit SHA' for merge commits in Bitbucket Data Center push events has been removed from the
ParsePayloadfunction. - Test Code Cleanup: Associated test code, including a specific test case for
on-path-changeon PR merge and relatedwantSHAassertions, has been removed fromparse_payload_test.goandbitbucket_datacenter_pull_request_test.go. - E2E Test Simplification: The
CreatePRfunction in the Bitbucket Data Center E2E test suite has been simplified by removing theBaseBranchoption and hardcoding 'main' as the base branch for PR creation. TheTearDownfunction now accepts a PR ID instead of a full PR object.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in pull request comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
pipelines-as-code
bot
added
bitbucket-datacenter
go
There was a problem hiding this comment.
Code Review
This pull request reverts a previous commit that introduced a fix for handling on-path-change during pull request merges in Bitbucket Data Center. The changes primarily involve removing the logic that identified the correct commit SHA for merge commits. The corresponding unit and end-to-end tests have also been removed, and related test helper functions have been cleaned up and simplified. The revert appears to be clean and complete.
|
/lgtm Please link the ticket and update release notes |
![pipelines-as-code[bot]](https://avatars.githubusercontent.com/in/112592?s=60&v=4){kind=small}
There was a problem hiding this comment.
Congrats @zakisk your PR Has been approved 🎉
✅ Pull Request Approved
Approval Status:
- Required Approvals: 1
- Current Approvals: 1
👥 Reviewers Who Approved:
| Reviewer | Permission Level | Approval Status |
|---|---|---|
| @aThorp96 | admin |
✅ |
📝 Next Steps
- Ensure all required checks pass
- Comply with branch protection rules
- Request a maintainer to merge using the
/mergecommand (or merge it
directly if you have repository permission).
Automated by the PAC Boussole 🧭
|
/retest linters |
|
Please dont' do lazy revert like this, explains the reason why we are reverting. and the jira ticket about it |
This reverts commit f9fd3f3. Reason for Revert: The previous fix for SRVKP-7432 introduced an unexpected bug (SRVKP-8616) where the `{{ revision }}` dynamic variable in Pipelines as Code no longer fetches the correct, newest merge commit in Bitbucket Data Center. Instead, it was fetching the last commit of the source branch. Customer feedback indicates this change breaks their CI/CD workflow, as their teams rely on the merge-commit ID and do not wish to alter their git strategies. This is a critical blocker for them, preventing them from upgrading to versions 1.19.x and accessing other important fixes, such as the one for CAP-724. This revert restores the previous behavior, ensuring the `{{ revision }}` variable correctly references the merge commit. A long-term solution to address both issues (SRVKP-7432 and SRVKP-8616) will be developed in separate, newly-created tickets (SRVKP-8619 and SRVKP-8620). Fixes: SRVKP-8616 Related: SRVKP-7432, SRVKP-8619, SRVKP-8620 Signed-off-by: Chmouel Boudjnah <chmouel@redhat.com>
chmouel
force-pushed
the
revert-bitbucket-fix-pr-2145
branch
from
e22324a to
b8a5204
Compare
chmouel
changed the title
|
since it's an urgency I fixed the PR and add a proper commit.... @zakisk |
|
/test linters |
|
/cherry-pick release-v0.35.x |
|
@savitaashture we don't use prow on pac |
4 participants
Description of the Change
This pull request reverts the changes made in a previous fix for Bitbucket Data Center (commit
f9fd3f3f7ba68db845ec2a0b723798ed812f4a1e). The original change aimed to fix an issue where on-path-change annotations were not working on merge commits, but it introduced a new, critical regression.The primary issue with the previous fix is that the
{{ revision }}dynamic variable no longer correctly fetches the ID of the newest merge commit when a pull request is merged on Bitbucket Data Center. Instead, it was incorrectly fetching the commit ID of the source branch's HEAD. This behavior is a blocker for a customer who relies on the merge-commit ID for their workflows and cannot change their existing git strategies.By reverting the change, we restore the expected behavior of the
{{ revision }}variable, allowing customers to upgrade to the latest versions. The underlying problem ofon-path-changenot working with Bitbucket merge commits (SRVKP-7432) is a separate issue that will be addressed in a future, more comprehensive fix.Linked Jira Tickets
revisionnot fetch to newest commitType of Change
Testing Strategy
Reviewer Checklist
📝 Description of the Change
🔗 Linked GitHub Issue
Fixes #
👨🏻 Linked Jira
🚀 Type of Change
fix:)feat:)feat!:,fix!:)docs:)chore:)refactor:)enhance:)deps:)🧪 Testing Strategy
✅ Submitter Checklist
fix:,feat:) matches the "Type of Change" I selected above.make testandmake lintlocally to check for and fix anyissues. For an efficient workflow, I have considered installing
pre-commit and running
pre-commit installtoautomate these checks.