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

Making HappyBase enabled/disabled checks into no-ops. #1997

Merged
merged 3 commits into from
Jul 19, 2016

Conversation

dhermes
Copy link
Contributor

@dhermes dhermes commented Jul 19, 2016

Fixes #1989.

/cc @mbrukman @tswast

@dhermes dhermes added the api: bigtable Issues related to the Bigtable API. label Jul 19, 2016
@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Jul 19, 2016
@tseaver
Copy link
Contributor

tseaver commented Jul 19, 2016

LGTM pending Travis.

@dhermes dhermes assigned tswast and mbrukman and unassigned tseaver Jul 19, 2016
@dhermes
Copy link
Contributor Author

dhermes commented Jul 19, 2016

Thanks @tseaver, assigned @tswast and @mbrukman before the merge

@mbrukman
Copy link
Contributor

LGTM

Are there any concerns about non-actionable warning messages, since there's nothing a user can do to remove the warnings, short of not calling the functions? I guess since these actions are administrative rather than data-oriented, they should be called rather rarely, so it shouldn't clog up their logs too much, and they ought to know that they're no-ops, so I think this is fine.

Thanks for making the change!

Also fixing some lint related errors (not using arguments
and not using self).
@dhermes
Copy link
Contributor Author

dhermes commented Jul 19, 2016

Fixed the lint breakage and added compact_table() as a no-op.

@mbrukman @tseaver PTAL?

@tseaver
Copy link
Contributor

tseaver commented Jul 19, 2016

LGTM

@dhermes
Copy link
Contributor Author

dhermes commented Jul 19, 2016

@mbrukman Implemented your updates to the language.

@mbrukman
Copy link
Contributor

LGTM, thanks!

@dhermes dhermes merged commit 24c4358 into googleapis:master Jul 19, 2016
@dhermes dhermes deleted the fix-1989 branch July 19, 2016 20:07
This was referenced Aug 3, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: bigtable Issues related to the Bigtable API. cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants