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

Rename resolve_type method #775

Closed
wants to merge 2 commits into from

Conversation

jkimbo
Copy link
Member

@jkimbo jkimbo commented Jun 21, 2018

Fixes #50

@jkimbo jkimbo requested a review from syrusakbary June 21, 2018 16:26
@syrusakbary
Copy link
Member

I like this, however, let's make sure we write this down as a breaking change on the release, so people are aware the name has to be changed from resolve_type to _resolve_type.
(It will also require a minor version bump 2.2.0)

@honi
Copy link

honi commented Oct 18, 2018

Any update on this?

Copy link
Member

@mvanlonden mvanlonden left a comment

Choose a reason for hiding this comment

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

Approving but wait until ready for 3.0 release, should be merged alongside #738

@jkimbo jkimbo added the breaking change Pull requests that introduce a breaking change label Mar 16, 2019
@ProjectCheshire ProjectCheshire added the 3.0 Fix/Release version 3.0 label Jun 2, 2019
@stale
Copy link

stale bot commented Jul 29, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the wontfix label Jul 29, 2019
@jkimbo jkimbo self-assigned this Jul 30, 2019
@stale stale bot removed the wontfix label Jul 30, 2019
@jkimbo jkimbo removed their assignment Jul 30, 2019
@stale
Copy link

stale bot commented Sep 28, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the wontfix label Sep 28, 2019
@stale stale bot closed this Oct 5, 2019
@jkimbo jkimbo mentioned this pull request Jan 29, 2020
8 tasks
@jkimbo
Copy link
Member Author

jkimbo commented Jun 24, 2020

For completeness the decision was taken to not make this change for the following reasons:

  1. the workaround for it is quite easy:
class MyInterface(Interface):
    type_ = String(name="type")
    def resolve_type_(_, info):
        return "foo"
  1. _resolve_type is a bit ugly

  2. I think we should be moving towards a @field decorator syntax for fields + resolvers. I’ve been using it my latest project and I quite like it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3.0 Fix/Release version 3.0 breaking change Pull requests that introduce a breaking change wontfix
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ObjectType.resolve_type classmethod makes it impossible to have a field named 'type'
6 participants