| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Sorry, something went wrong.
|
So I understand the argument that people may not be familiar with Guava, but in cases like this, it is obvious what the Guava code is doing, whereas the ByteBuffer code is somewhat obscure... @mziccard , who originally made the decision to not use Guava in the snippets? |
Sorry, something went wrong.
|
Guava is something developers should hear about. We might include a link Guava |
Sorry, something went wrong.
|
If we don't want to include Guava in these snippets (the imports aren't visible in the included javadocs), I could make a parallel sample in java-docs-samples so we can show the imports in the cloud docs. |
Sorry, something went wrong.
|
@jgeewax @anthmgoogle JJ / Anthony - do you have an opinion? As I mentioned earlier, I think it's ok to use guava in samples for the obvious stuff. |
Sorry, something went wrong.
|
My weak opinion is that allowing guava seems OK. My reasoning would be that it is highly regarded and if you need the utility in the sample, it will avoid either polluting the sample with more code in a different problem domain, or using an even more obscure library (much like @garrettjonesgoogle's case above). However, I'm concerned that @mziccard recommended against it and I be interested understanding more about the risks. However he is no longer employed by the project and may not weigh in. I would defer to @jgeewax on this issue. |
Sorry, something went wrong.
|
I think @aozarov might provide some useful input here - could you weigh in? |
Sorry, something went wrong.
|
I know that Guava makes snippets look smaller and possibly nicer but users do not necessarily know how to use it. Purpose of snippets is to show how our code works in the most general environment possible. Users might be more familiar with other libraries such as apache-commons (or they might be using google-cloud from java8) and I believe we should not teach the guava-dependent way of using our library. In particular, I am against using ByteStreams.copy. It is a nice shortcut, we can all agree on that, but it does not serve the purpose of showing how a writer works. Users familiar with Guava already know about that method, it should not be our goal to show Guava shortcuts for handling channels. |
Sorry, something went wrong.
|
I am also in the opinion that we should not use third-party libraries in the API layer and avoid them in samples. I would definitely prefer using "Cg0NDg0=" instead of BaseEncoding.base64().encode(new byte[]{0xA, 0xD, 0xD, 0xE, 0xD}). I can see how ByteStreams.copy(csvReader, writer); makes the example simpler by eliminating some "boilerplate" code however it can also confuse people that are unfamiliar with guava and may not understand were this method is coming from. In most cases I think we can achieve good enough result even without using thirdparty libraries. Even in this case, we can use Files.copy to make the example simpler. In other cases we may chose just to add a comment suggesting what should be done. |
Sorry, something went wrong.
| * .setFormatOptions(FormatOptions.csv()) | ||
| * .build(); | ||
| * TableDataWriteChannel writer = bigquery.writer(writeChannelConfiguration); | ||
| * ByteBuffer buffer = ByteBuffer.allocateDirect(16 * 1024); // 16 KB buffer |
|
Sigh - "Cg0NDg0=" is a lot less likely to be understood. |
Sorry, something went wrong.
Then add a comment saying that this a base64 encoded string. Also, FYI Java 8 added an API call for it. |
Sorry, something went wrong.
|
I find these arguments convincing, so I'll change my preference to avoid Guava here. |
Sorry, something went wrong.
We should avoid references to third-party libraries in snippets.
|
PTAL. I've incorporated your comments, which makes the sample much simpler without Guava. |
Sorry, something went wrong.
|
LGTM |
Sorry, something went wrong.
|
Changes Unknown when pulling ad29f3e on tswast:bigquery-snippets into ** on GoogleCloudPlatform:master**. |
Sorry, something went wrong.
…onfig to v1.5.1 (#1410) * build(deps): update dependency com.google.cloud:google-cloud-shared-config to v1.5.1 * 🦉 Updates from OwlBot post-processor See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>
…en-plugin to v1.7.0 (#1410)
…en-plugin to v1.7.0 (#1410)
…o v1-rev20240524-2.0.0 (#1410)
…o v1-rev20240524-2.0.0 (#1410)
| Back | FazBrowse Home | New Git URL |
The Guava classes may not be as familiar to people.
Re: #1407 (comment)
@mziccard FYI
@garrettjonesgoogle @lesv PTAL