-
Notifications
You must be signed in to change notification settings - Fork 0
fix: linter fixes #8
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
Conversation
4685e78
to
fa85c33
Compare
c1d42c0
to
1aa3880
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: 0 of 11 files reviewed, 1 unresolved discussion (waiting on @dopiera)
tensorflow_io/core/kernels/bigtable/bigtable_dataset_kernel.cc, line 136 at r1 (raw file):
private: mutex mu_;
anticipating the question, IDK why it used to compile but this mutex was not declared even though the Compute
method declaration above is using it:
void Compute(OpKernelContext* ctx) override TF_LOCKS_EXCLUDED(mu_)
¯_(ツ)_/¯
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: 0 of 11 files reviewed, 1 unresolved discussion (waiting on @dopiera)
tensorflow_io/core/kernels/bigtable/bigtable_dataset_kernel.cc, line 136 at r1 (raw file):
Previously, kboroszko (Kajetan Boroszko) wrote…
anticipating the question, IDK why it used to compile but this mutex was not declared even though the
Compute
method declaration above is using it:
void Compute(OpKernelContext* ctx) override TF_LOCKS_EXCLUDED(mu_)
¯_(ツ)_/¯
I only noticed it because the build for macOS was failing for this very reason.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: 0 of 11 files reviewed, 1 unresolved discussion (waiting on @dopiera and @kboroszko)
tensorflow_io/core/kernels/bigtable/bigtable_dataset_kernel.cc, line 136 at r1 (raw file):
Previously, kboroszko (Kajetan Boroszko) wrote…
I only noticed it because the build for macOS was failing for this very reason.
But why would we keep a mutex that we don't use? Please remove the annotations instead.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: 0 of 11 files reviewed, 1 unresolved discussion (waiting on @dopiera)
tensorflow_io/core/kernels/bigtable/bigtable_dataset_kernel.cc, line 136 at r1 (raw file):
Previously, dopiera (Marek Dopiera) wrote…
But why would we keep a mutex that we don't use? Please remove the annotations instead.
Right...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 10 of 11 files at r1, 1 of 1 files at r2, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @dopiera)
This PR fixes the files so they are consistent with CI linting and building and don't fail.
There is a few minor changes, mainly indents but also removing some
mutable
keywords that were not needed and adding a mutex in one place.This change is