Skip to content

Switching to the new version of shared library has broken Bitbucket's "Ready to Merge" category #4191

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

Closed
sergeibbb opened this issue Mar 31, 2025 · 4 comments
Assignees
Labels
needs-verification Request for verification pending-release Resolved but not yet released to the stable edition

Comments

@sergeibbb
Copy link
Member

sergeibbb commented Mar 31, 2025

seems to be caused by #4128
fixed in https://github.com/gitkraken/provider-apis-package-js/pull/231
the ticket is waiting for approval and for having a new library version shipped.

Description

It's Bitbucket.org
Image

Fixed in:

  • gitkraken/provider-apis-package-js#231
  • and 935263a

Verification steps

Have a PR that should appear in "Ready to merge":

  • you are the author
  • someone else has reviewed and approved

Before the fix: it's in Other

After the fix: it's in Ready to merge

System info

GitLens Version

16.3.3 pre-release

VS Code Version

Version: 1.99.0-insider
Commit: 99c9c6c8eb0aef3fce659b0fac1ff3130c4e34a4
Date: 2025-03-28T21:28:51.696Z (2 days ago)
Electron: 34.3.2
ElectronBuildId: 11161073
Chromium: 132.0.6834.210
Node.js: 20.18.3
V8: 13.2.152.41-electron.0
OS: Darwin arm64 24.3.0

Git Version

No response

Logs, Screenshots, Screen Captures, etc

No response

@sergeibbb sergeibbb added the triage Needs to be looked at label Mar 31, 2025
@sergeibbb sergeibbb added this to the 17.0-patch milestone Mar 31, 2025
@sergeibbb sergeibbb self-assigned this Mar 31, 2025
@eamodio eamodio modified the milestones: 17.0-patch, 17.1 Apr 1, 2025
@d13 d13 removed this from the 17.1 milestone Apr 9, 2025
@sergeibbb sergeibbb removed the triage Needs to be looked at label Apr 10, 2025
@sergeibbb
Copy link
Member Author

Hi @jdgarcia , @gitkraken-jacobw , @axosoft-ramint

Here (and maybe in other places) in the categorization method we require a PR to be in Mergeable state in order to put it into the mergeable category:
https://github.com/gitkraken/provider-apis-package-js/blob/6957f294cef3ea0650b1bb438307be6599d8b79a/src/providerUtils/gitProvider.ts#L144-L148

But here in Bitbucket (and maybe in other providers) we put the mergeable state to Unknown because it's not supported:
https://github.com/gitkraken/provider-apis-package-js/blob/6957f294cef3ea0650b1bb438307be6599d8b79a/src/providers/bitbucket/bitbucket.ts#L153C50-L153C54

As a result, Bitbucket PRs can never be shown in READY_TO_MERGE category.

Therefore, we need to do one of the following:

  1. Allow a PR be in Unknown mergeable state to appear in READY_TO_MERGE.
  2. Set this field in providers where this field is not supported to Mergeable rather than to Unknown.

What option do you think is better?

@axosoft-ramint
Copy link
Contributor

@sergeibbb We may want a client-side override on the categorization where we think is appropriate. Or we may want to see if we can get a pulse on a Bitbucket PR's mergeability from the other properties we do have access to, and maybe set mergeable state if we determine it from those other properties. I definitely suggest against assuming a PR is mergeable if its state is unknown as a general rule. So I would lean towards 2 but only in cases where we have enough context from other properties of the PR to determine that it is indeed mergeable, not as a default.

@sergeibbb
Copy link
Member Author

Re client-side override

It's not only GitLens's problem. GK.dev shows this PR incorrectly too. To me it's a sign that it should be fixed in the library:

Image

Does anybody mind if I go with n.2. "Set Bitbucket to Mergeable rather than to Unknown"? @jdgarcia , @gitkraken-jacobw , @axosoft-ramint ?

@axosoft-ramint
Copy link
Contributor

Yeah, the more I think about it, the more it makes sense as a general rule. I think I'm more on the side of 2 now, with the assumption that if you couldn't actually merge, you would get an error from the provider anyway.

@github-actions github-actions bot added needs-verification Request for verification pending-release Resolved but not yet released to the stable edition labels Apr 16, 2025
@sergeibbb sergeibbb reopened this Apr 22, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-verification Request for verification pending-release Resolved but not yet released to the stable edition
Projects
None yet
Development

No branches or pull requests

4 participants