Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Initial Tableau Reading Support #10733
Initial Tableau Reading Support #10733
Changes from 23 commits
e9fa255
d8136f4
143c2bd
5d5bf36
297ca24
d20ae0b
b2dd092
0c55cba
6ac10c1
68c6d1c
379f9a6
aa2576f
3286ba4
7803fc8
1133948
eab5a77
2adffe1
8139009
cd4c0a3
a1f52e7
a0c5357
30f3c47
6594c50
35828c3
3708118
4c4991a
00546cc
f5164a6
3bedc2d
a782089
dfb4b7c
fa6eaf5
6bb721c
fe90653
872384f
2d1d7c0
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
Should we use
ManagedLogger
?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.
In general I didn't see the need for any logging. Somehow CI doesn't like my solution and we can't reproduce locally
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.
As you prefer. I think it's useful to see a log for this (at least on debug level) to be able to tell what's happening. I'd especially appreciate a message right before we start a download of the 200Mb file - for fiber connections that's moments, but when you are developing on a train, getting a message explaining to you why the build process just started to block for 5 minutes is actually precious information.
I think it's better to do this via
ManagedLogger
though, as it allows users to manage log levels and gathers logs in one place - print is going to be completely separate from it.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.
Forgot to say but I think ideally we should add an entry about
HyperAPI
here. Can be added by modifyingnotice-header
.But current is probably OK too since we do have the
COPYRIGHT-HYPERAPI
file, so feel free to ignore.