-
Notifications
You must be signed in to change notification settings - Fork 285
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 handling of GHE without merge queue support #926
Conversation
Summary: facebook#830 and facebook#906 reported sapling not working for old Github Enterprise versions since their graphql schema doesnt include merge queue. This adds a query to detect if merge queue is supported in the graphql version. Then, it will issue a different "YourPullRequestsQuery" depending on if merge queue is supported. I considered a few other approaches: - Just detect the error message and retry without merge queue field - a little gross to extract from the error - Use "@include" instead of an entirely separate query - didn't seem to count as a valid query still via graphiql at least (unsure if the query client here would strip out the unused field though) Test Plan: Haven't had a chance to test yet, will try and validate that both versions work and that the detection logic works.
@facebook-github-bot has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
@muirdm @zzl0 @kavehahmadi60 |
This looks good to me and works in my testing, but I don't have a github enterprise repo without merge support. Would love help validating that flow works as expected. |
This pull request has been merged in 4aa5697. |
This should be available now in sapling-scm VS Code extension version 0.1.54, please let me know if this works for folks using GHE! |
I am not using VS Code extension but using isl. Is there a way to patch this to ISL if I don't want to run ISL locally |
I think you either need to build from source or wait for the next Sapling release, which is hopefully coming soon |
I tried it locally build from source. Although the error is gone, I am still seeing trouble integrating with GHE - a lot of spinners. Not sure if it is because how I patched it locally doesn't work. Do you know who should reach out for the next release? |
Fix handling of GHE without merge queue support
Summary:
#830 and #906 reported sapling not working for old Github Enterprise versions since their graphql schema doesnt include merge queue.
This adds a query to detect if merge queue is supported in the graphql version. Then, it will issue a different "YourPullRequestsQuery" depending on if merge queue is supported.
I considered a few other approaches:
Test Plan:
Haven't had a chance to test yet, will try and validate that both versions work and that the detection logic works, but wanted to put up the PR first at least so someone could potentially take over if needed