Skip to content

Replace shurcool/graphql with cli/shurcool-graphql#4750

Merged
samcoe merged 1 commit intotrunkfrom
replace-shurcool
Nov 18, 2021
Merged

Replace shurcool/graphql with cli/shurcool-graphql#4750
samcoe merged 1 commit intotrunkfrom
replace-shurcool

Conversation

@samcoe
Copy link
Contributor

@samcoe samcoe commented Nov 17, 2021

Replace shurcool/graphql module with cli/shurcool-graphql.
cc #1389

@samcoe samcoe self-assigned this Nov 17, 2021
@samcoe samcoe marked this pull request as ready for review November 17, 2021 20:59
@samcoe samcoe requested a review from a team as a code owner November 17, 2021 20:59
@samcoe samcoe requested review from mislav and removed request for a team November 17, 2021 20:59
teams, err := OrganizationTeams(client, repo)
// TODO: better detection of non-org repos
if err != nil && !strings.HasPrefix(err.Error(), "Could not resolve to an Organization") {
if err != nil && !strings.Contains(err.Error(), "Could not resolve to an Organization") {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was already super fragile checking an error message string, luckily we had tests to catch this since the new library slightly changes the error message format. I did a quick search and I think these are the only two places that should be effected by that.

@samcoe samcoe requested a review from vilmibm November 17, 2021 21:04
Copy link
Contributor

@mislav mislav left a comment

Choose a reason for hiding this comment

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

Thanks for doing this!!

@samcoe samcoe merged commit 2580a63 into trunk Nov 18, 2021
@samcoe samcoe deleted the replace-shurcool branch November 18, 2021 17:10
@VictorBatta VictorBatta mentioned this pull request Dec 4, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants