这是indexloc提供的服务，不要输入任何密码
Skip to content

merge types with same structure in remote schema (fix #1112) #1145

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 10 commits into from
Dec 12, 2018
Merged

merge types with same structure in remote schema (fix #1112) #1145

merged 10 commits into from
Dec 12, 2018

Conversation

@ecthiender
Copy link
Contributor

@ecthiender ecthiender commented Nov 30, 2018 •
edited
Loading

Description

If the remote schema has types with same structure and same name, they are merged now.

What component does this PR affect?

  • Server
  • Console
  • CLI
  • Docs
  • Community Content
  • Build System

Requires changes from other components? If yes, please mark the components:

  • Server
  • Console
  • CLI
  • Docs
  • Community Content
  • Build System

Related Issue

#1112

Solution and Design

We have a TyEq typeclass. Declared instances of TyEq on various TypeInfo types, which compares the structure except name and TypeLoc.
ObjTyInfo is compared by name and its fields.

Type

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Docs update
  • Community content

Checklist:

  • I have read the contributing guide and my code conforms to the guidelines.
  • This change requires a change in the documentation.
  • I have updated the documentation accordingly.
  • I have added required tests.

@ecthiender ecthiender added the c/server Related to server label Nov 30, 2018
@ecthiender ecthiender requested a review from 0x777 November 30, 2018 04:27
@hasura-bot
Copy link
Contributor

hasura-bot commented Nov 30, 2018

Review app for commit e73f8ee deployed to Heroku: https://hge-ci-pull-1145.herokuapp.com
Docker image for server: hasura/graphql-engine:pull1145-e73f8ee

@hasura-bot
Copy link
Contributor

hasura-bot commented Dec 7, 2018

Review app for commit eb4a787 deployed to Heroku: https://hge-ci-pull-1145.herokuapp.com
Docker image for server: hasura/graphql-engine:pull1145-eb4a787

0x777
0x777 reviewed Dec 12, 2018
View reviewed changes
server/src-lib/Hasura/GraphQL/Validate/Types.hs Outdated
, fromSchemaDocQ
, TypeMap
, TypeLoc (..)
--, TyEq (..)
Copy link
Member

@0x777 0x777 Dec 12, 2018

Choose a reason for hiding this comment

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

remove the comment

0x777
0x777 reviewed Dec 12, 2018
View reviewed changes
server/src-lib/Hasura/GraphQL/Validate/Types.hs Outdated
type EqProps a
getEqProps :: a -> EqProps a

typeEq :: (Show (EqProps a), EquatableGType a, Eq (EqProps a)) => a -> a -> Bool
Copy link
Member

@0x777 0x777 Dec 12, 2018

Choose a reason for hiding this comment

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

Why the Show constraint?

Copy link
Contributor Author

@ecthiender ecthiender Dec 12, 2018

Choose a reason for hiding this comment

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

I was debugging. Will remove it.

@hasura-bot
Copy link
Contributor

hasura-bot commented Dec 12, 2018

Review app for commit 9ee13e2 deployed to Heroku: https://hge-ci-pull-1145.herokuapp.com
Docker image for server: hasura/graphql-engine:pull1145-9ee13e2

0x777
0x777 reviewed Dec 12, 2018
View reviewed changes
server/src-lib/Hasura/GraphQL/Validate/Types.hs
, _iotiLoc :: !TypeLoc
} deriving (Show, Eq, TH.Lift)

instance EquatableGType InpObjTyInfo where
Copy link
Member

@0x777 0x777 Dec 12, 2018

Choose a reason for hiding this comment

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

So we are comparing the descriptions of the fields in InpObjTyInfo?

0x777
0x777 previously approved these changes Dec 12, 2018
View reviewed changes
@hasura-bot
Copy link
Contributor

hasura-bot commented Dec 12, 2018

Review app for commit e549b31 deployed to Heroku: https://hge-ci-pull-1145.herokuapp.com
Docker image for server: hasura/graphql-engine:pull1145-e549b31

@0x777
Copy link
Member

0x777 commented Dec 12, 2018

@ecthiender This also closes #1135?

@0x777 0x777 merged commit 77cbf12 into hasura:master Dec 12, 2018
@hasura-bot
Copy link
Contributor

hasura-bot commented Dec 12, 2018

Review app https://hge-ci-pull-1145.herokuapp.com is deleted

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@0x777 0x777 0x777 approved these changes

Assignees

No one assigned

Labels

c/server Related to server

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@ecthiender @hasura-bot @0x777