Skip to content

add sharing parameter for saveplotlyconfig #104

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 5 commits into from
Jul 15, 2016
Merged

Conversation

yankev
Copy link
Contributor

@yankev yankev commented Jul 14, 2016

@BRONSOLO
Copy link
Member

@yankev looks good. Did you test this with various settings / does it update + validate properly?

@@ -1,4 +1,4 @@
function saveplotlyconfig(plotly_domain,plotly_streaming_domain)
function saveplotlyconfig(plotly_domain,plotly_streaming_domain, sharing)
Copy link
Member

Choose a reason for hiding this comment

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

should we make the spacing here consistent too? just for looks 👗

@yankev
Copy link
Contributor Author

yankev commented Jul 15, 2016

Yep I tried a couple variations, and it seems to validate and update correctly. Only issue would be that if validation fails then the config file just be empty. We could keep this behaviour, or change the error to a warning, which would allow the other parameters to be written

@BRONSOLO
Copy link
Member

@yankev good point - could you make https://github.com/plotly/MATLAB-api/pull/104/files#diff-1a55e631c9c6db6c4e1372e0d2cc6f85R67 a simple warning instead of failing to save everything.

@yankev
Copy link
Contributor Author

yankev commented Jul 15, 2016

@BRONSOLO cool, just made that change.
@cldougl Should be good to review.

@BRONSOLO
Copy link
Member

cool - 💃 from me.

@cldougl
Copy link
Member

cldougl commented Jul 15, 2016

👍 I'll get the doc edit when you merge this @yankev

@yankev yankev merged commit 4bd4d92 into master Jul 15, 2016
BRONSOLO added a commit that referenced this pull request Jul 19, 2016
This reverts commit 4bd4d92, reversing
changes made to 4ddb8c3.
@BRONSOLO BRONSOLO deleted the save_config_sharing branch July 28, 2016 13:36
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