closes #132,#133 #134
closes #132,#133 #134
Conversation
| for _, var in node.arguments: | ||
| if var.name in required: | ||
| required.remove(var.name) | ||
| for name, _ in node.arguments: |
There was a problem hiding this comment.
could you check if this works with variables? See this fix I had to do sometime ago in a similar code e4a22f4
I'd suggest to keep the old behavior if it's a variable, then: name, maybe_var and you check hasattr(maybe_var, 'name') to use it overriding name
| @@ -262,7 +262,9 @@ def format_selection_set_inline_fragment(self, parent, type_condition, | |||
| children, lines, idx): | |||
| sel = self.selection_name(parent, '_as__%s' % type_condition, idx) | |||
| idx += 1 | |||
| lines.append('%s = %s.__as__(%s)' % (sel, parent, type_condition)) | |||
| full_type_condition = str(self.schema_name) + "." + type_condition | |||
There was a problem hiding this comment.
We should use format_type_usage() instead, I don't think we should need the wrapper version, but that would be a declaration error, on our side (code generation), we better use it to keep consistent.
There was a problem hiding this comment.
Just so I understand you correctly: You suggest to use another format_type_usage() instead, e.g. format_type_condition()?
There was a problem hiding this comment.
There is a method called self.format_type_usage(typ) that will handle the schema name prefix.
|
Hi @maaft , thanks for your patches. I'm bit picky with the commits and git history, so I'd kindly ask for you to create 2 commits, one with the fix to #132 and another for #133. They should be in the same PR, but only 2 commits, each already incorporating the style fixes. For #132, I have a suggestion to use However for #133, I think we need to cope with both cases... I thought I fixed it... and I did, but I forgot to merge, see #126 :-( my bad |
|
also, please git-rebase on the newest master |
|
closing this one as it was fixed in b208ca6 please reopen if something is still missing |
No description provided.