Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Add revision settings to Update Site#187

Merged
t8y8 merged 4 commits intotableau:developmenttableau/server-client-python:developmentfrom
t8y8:186-feature-revision-settingst8y8/server-client-python:186-feature-revision-settingsCopy head branch name to clipboard
May 10, 2017
Merged

Add revision settings to Update Site#187
t8y8 merged 4 commits intotableau:developmenttableau/server-client-python:developmentfrom
t8y8:186-feature-revision-settingst8y8/server-client-python:186-feature-revision-settingsCopy head branch name to clipboard

Conversation

@t8y8
Copy link
Collaborator

@t8y8 t8y8 commented May 10, 2017

Addresses #186

(And helps me with some internal stuff)

Updated tests and manually tested internally

@t8y8 t8y8 requested a review from graysonarts May 10, 2017 00:48
@t8y8 t8y8 self-assigned this May 10, 2017
@LGraber
Copy link
Contributor

LGraber commented May 10, 2017

How come you didn't added a getter / setter for revisionLimit? How come these values are not read from get requests? I haven't checked the code to see if we are returning them but it seems like we would be.

@t8y8
Copy link
Collaborator Author

t8y8 commented May 10, 2017

I didn't change the model, it was already there so I didn't even check, I just added it to the serializer

@LGraber
Copy link
Contributor

LGraber commented May 10, 2017

Huh. Not good that we have a partial implementation like that. Oh well. Thanks for fixing! For your CL, though, it looks like we are simply naming the member "revision_limit" and not using a getter / setter. This doesn't match any of our other properties. Can you fix that? We might also debate on requiring revision_history_enabled to be true to set it but for now, I don't think that matters. Besides that it looks great.

Copy link
Contributor

@graysonarts graysonarts left a comment

Choose a reason for hiding this comment

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

🚀 🎸 🇫🇷

Copy link
Contributor

@LGraber LGraber left a comment

Choose a reason for hiding this comment

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

rocket

@t8y8 t8y8 merged commit 6f9dc37 into tableau:development May 10, 2017
@t8y8 t8y8 deleted the 186-feature-revision-settings branch May 10, 2017 21:01
t8y8 added a commit to t8y8/server-client-python that referenced this pull request Jun 28, 2017
Revision History settings were present in the SiteItem model but not actually serialized and didn't have setters/getters. I've updated that and enhanced the is_int decorator to allow exemptions in the case of sentinels that fall outside the normal range.
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.

3 participants

Morty Proxy This is a proxified and sanitized view of the page, visit original site.