Skip to content

Conversation

dsauve-adsk
Copy link
Contributor

Preview of the changes ( the same changes have been applied to the two functions )
Screenshot 2023-03-13 at 9 44 23 AM

Copy link
Contributor

@carlos-villavicencio-adsk carlos-villavicencio-adsk left a comment

Choose a reason for hiding this comment

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

I'd suggest adding indentation to multiline lists and dicts as black would format them. This will improve readability.

Comment on lines 853 to 856
[
{'field_name':'foo', 'direction':'asc'},
{'field_name':'bar', 'direction':'desc'}
]
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe the indentation can be improved:

Suggested change
[
{'field_name':'foo', 'direction':'asc'},
{'field_name':'bar', 'direction':'desc'}
]
[
{'field_name':'foo', 'direction':'asc'},
{'field_name':'bar', 'direction':'desc'},
]

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just push that suggestion...
Screenshot 2023-03-13 at 4 46 13 PM

@dsauve-adsk dsauve-adsk force-pushed the ticket/SG-30217_fix_text_overflow branch from 4773de4 to 190ce36 Compare March 13, 2023 20:47
Copy link
Contributor

@carlos-villavicencio-adsk carlos-villavicencio-adsk left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@NorberMV NorberMV left a comment

Choose a reason for hiding this comment

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

LGTM !

@dsauve-adsk dsauve-adsk merged commit 7651d8a into master Mar 21, 2023
@dsauve-adsk dsauve-adsk deleted the ticket/SG-30217_fix_text_overflow branch March 21, 2023 15:17
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.

3 participants