Skip to content

docs: correct delete_obj_default_permissions sig #1483

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

Merged
merged 1 commit into from
Oct 11, 2024

Conversation

jorwoods
Copy link
Contributor

@jorwoods jorwoods commented Oct 1, 2024

Closes #1241

The docs were incorrect as to what arguments
delete_*_default_permissions accepted. This corrects the docs to show that the methods accept an object and not an iterable of objects.

Closes tableau#1241

The docs were incorrect as to what arguments
delete_*_default_permissions accepted. This corrects the docs to show
that the methods accept an object and not an iterable of objects.
@jorwoods jorwoods force-pushed the jorwoods/del_default_perms branch from e273d95 to 53bb858 Compare October 11, 2024 02:10
Copy link
Contributor

@jacalata jacalata left a comment

Choose a reason for hiding this comment

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

Very nice. Do you think it's worth updating the code to add list handling, or is one at a time enough for most use cases?

@jacalata jacalata merged commit 8bdaa67 into tableau:gh-pages Oct 11, 2024
2 checks passed
@jorwoods jorwoods deleted the jorwoods/del_default_perms branch October 11, 2024 21:35
@jorwoods
Copy link
Contributor Author

I am conflicted on that. It could be worth doing, but the underlying endpoint only supports one permission at a time. So it would be "hiding" multiple calls being made. I think the user performing the call in a loop is simple enough.

It may still be desired to support accepting a list from the perspective of method symmetry though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants