-
Notifications
You must be signed in to change notification settings - Fork 81
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
Allow descriptions schema #1305
Conversation
if isinstance(self.type, Enum): | ||
return f'{self.name} : {self.type.name}' | ||
return f'{description_str}{self.name} : {self.type.name}' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, do we want to store \n in description itself?
IMHO, it's better to format it in place
'{description_str}\n{self.name} : {self.type.name}'
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
well we do not need \n
if there is a description - so the new line is tied to the description a bit
I have not played around with how descriptions can be interpreted - I can play around a bit to see if can keep all on same line but if not I will keep it as is
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From my research - the standard seems to be adding comments on a new line above the type or argument you are describing (ex: https://medium.com/@krishnaregmi/how-to-write-comments-in-a-graphql-schema-to-enhance-auto-generated-documentation-c0047125ea24)
I think best to follow the same
@@ -21,7 +21,7 @@ | |||
|
|||
SearchGlossary = gql.QueryField( | |||
name='searchGlossary', | |||
doc='Search glossary ', | |||
description='Search glossary ', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here, after the transformation described above, we will have both space and \n.
Is it ok?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not sure I follow but the extra space is not a problem - will have a later PR that adds descriptions to all of our GQL queries, mutations, etc. and can clean up this description then
Feature or Bugfix
Detail
Allow dataset admin role glue permissions for dataset crawler
Allow GQL Types, Arguments, Inputs, and Fields to take in
description
instance variable and write GQL schema including the description docstringRelates
Security
Please answer the questions below briefly where applicable, or write
N/A
. Based onOWASP 10.
fetching data from storage outside the application (e.g. a database, an S3 bucket)?
eval
or similar functions are used?By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.