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

ChangesetGet without include_discussion=True - maybe return 'discussion': None not 'discussion': []? #163

Closed
matkoniecz opened this issue Apr 2, 2024 · 2 comments · Fixed by #164

Comments

@matkoniecz
Copy link
Contributor

matkoniecz commented Apr 2, 2024

Currently calling ChangesetGet without include_discussion=True will give you dict with 'discussion': [] what is a bit confusing. I (again) got confused why also changesets with some comments also get empty list.

Maybe 'discussion': None would be more clear?

It may break some data consumers but in almost all cases any code that tried to iterate over not actually fetched list of comments was broken anyway.

(I can imagine some exotic case of code distinguishing between empty list and not empty list for changeset known to have comments to detect which version of fetch was performed, but that is at best exotic..)

And big thanks for this library, it is quite useful! And such very minor issues are really rare! I hope feedback like this issue is welcome.

@metaodi
Copy link
Owner

metaodi commented Apr 2, 2024

Your feedback is very welcome, thank you!

How about omitting the discussion key altogether? Since it was not requested anyways.

@matkoniecz
Copy link
Contributor Author

Oh, that would be even better and less mysterious.

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 a pull request may close this issue.

2 participants