Conversation
cliAutomation
added
the
external
|
Hi! Thanks for the pull request. Please ensure that this change is linked to an issue by mentioning an issue number in the description of the pull request. If this pull request would close the issue, please put the word 'Fixes' before the issue number somewhere in the pull request body. If this is a tiny change like fixing a typo, feel free to ignore this message. |
There was a problem hiding this comment.
This looks great for GitHub.com. Unfortunately, we cannot merge it as-is because it would break gh release list for GitHub Enterprise < 3.1, as they didn't yet have the isLatest field in their GraphQL schema, so for them this updated query would result in an error.
The way we work around this in other parts of gh is that we query the schema for whether a certain field is available. For example, for pull requests:
Line 274 in 0607ce5
Unfortunately, this complicates the whole query approach, since now we need dynamic instead of static queries, so we cannot anymore use graphql.NewClient since that library only supports static queries. I don't expect you to update this PR for a different query approach because we don't have any documentation on how to do it. If you give us some time, someone from our team will get around to pushing updates to this PR to make it compatible with GHE 👍
|
Thanks for the explanation, I didn't realize that was a consideration. Feel free to push to this or close and create a fresh pr as you see fit, thanks. |
mislav
left a comment
mislav
left a comment
There was a problem hiding this comment.
I've pushed a test fix for this and approved the change since GitHub Enterprise 3.0 is no longer supported by GitHub and thus I feel we can break it from gh as well. I've considered adding conditional GraphQL queries here, but it would be a lot of work for little payoff.
3 participants
Based on the history for this utility
IsLatestdidn't exist when it was written as it was added in https://docs.github.com/en/graphql/overview/changelog#schema-changes-for-2021-01-19.Fixes #4922
to match what is displayed on https://github.com/elastic/elasticsearch/releases
