Skip to content
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: as gateway resolver key should handle digits (#880) #881

Merged
merged 1 commit into from
Oct 2, 2022
Merged

fix: as gateway resolver key should handle digits (#880) #881

merged 1 commit into from
Oct 2, 2022

Conversation

marcobeca
Copy link
Contributor

@marcobeca marcobeca commented Sep 29, 2022

Fixes #880

@mcollina
Copy link
Collaborator

@jonnydgreen the replacement was added by you in #754, however the tests are not failing.

@jonnydgreen
Copy link
Contributor

@jonnydgreen the replacement was added by you in #754, however the tests are not failing.

Interesting, I'll take a look this evening!

@jonnydgreen
Copy link
Contributor

Apologies, something came up so I wasn't able to have a look last night - I've blocked some time over the weekend so I'll have a look then if that's okay!

@jonnydgreen
Copy link
Contributor

jonnydgreen commented Oct 1, 2022

@mcollina am I missing something here, it seems that the replacement (assuming you're talking about the _IDX_ replacement) wasn't added in the PR you linked? I only added the type suffix in #754 , which is not changed in this PR?

Screenshot 2022-10-01 at 17 25 49

However, either way, I would expect the tests to fail, yep

@mcollina
Copy link
Collaborator

mcollina commented Oct 2, 2022

Ah,yeah! I made a mistake. I'll keep looking with git blame and see where it come from!

@mcollina
Copy link
Collaborator

mcollina commented Oct 2, 2022

The _IDX_ replacement was added in #130. I think it's safe to land this change.

Copy link
Collaborator

@mcollina mcollina left a comment

Choose a reason for hiding this comment

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

lgtm

@mcollina mcollina merged commit ca12479 into mercurius-js:master Oct 2, 2022
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.

As gateway resolverKey should handle digits
3 participants