🔍 Examples of Generated Samples for Code Review [DO NOT MERGE] by beccasaurus · Pull Request #6543 · googleapis/google-cloud-java

beccasaurus · 2019-10-23T00:43:03Z

Pull Request to facilitate code review of GAPIC SampleGen rendered syntax. Do not merge.

This Pull Request contains two code samples which were generated using the GAPIC generator:

Natural Language API – "Analyze Sentiment" (language_sentiment_text) (config YAML)
Speech-to-Text API – "Long-Running Transcribe" (speech_transcribe_async) (config YAML)

^--- Actually, I added a bunch of samples. But call your attention to these ones 🙂

These code samples demonstrate a few features of the code sample generator:

Overall code sample structure (imports, layout, etc)
Calling a synchronous API endpoint and printing fields from the API response
Calling a long-running API, waiting for completion, and printing fields from API response
Using local files with code samples, specifically a file to use for a bytes request field

You can find the existing, published hand-written versions of these code samples here:

📙 Analyzing Sentiment in a String
📘 Transcribing long audio files (shows a different version of this sample using Cloud Storage)

For Java, existing hand-written code samples can be found in the java-docs-samples repository.

The goal is for these generated code samples to meet the expectations of hand-written samples.

Language experts, please TAL! 🧐

/cc @kurtisvg @nnegrey @chingor13 @lesv @averikitsch
/cc @googleapis/samplegen

This is one of seven Pull Requests being sent for code review of rendered generated sample syntax.

Language	Pull Request
#️⃣ C#	googleapis/google-cloud-dotnet#3650
🐹 Go	googleapis/google-cloud-go#1634
☕️ Java	#6543
🚀Node.js	googleapis/google-cloud-node#2953
🐘 PHP	googleapis/google-cloud-php#2420
🐍 Python	googleapis/google-cloud-python#9514
💎 Ruby	googleapis/google-cloud-ruby#4264

📓 Internal doc for context

chingor13 · 2019-10-23T17:53:24Z

I would really like use to mvn test to execute the tests. This would mean for each sample, generating a junit test to match that test which could execute the sample's static method and verify whatever the sample-tester test is doing.

The long-term plan is to have all samples (generated and hand-written) live in the samples/ folder and we want to ensure test coverage. We also want to make it approachable for java developers who may not be familiar with python to install python tools to run tests.

kurtisvg · 2019-10-23T18:06:47Z

I would really like use to mvn test to execute the tests. This would mean for each sample, generating a junit test to match that test which could execute the sample's static method and verify whatever the sample-tester test is doing.

The long-term plan is to have all samples (generated and hand-written) live in the samples/ folder and we want to ensure test coverage. We also want to make it approachable for java developers who may not be familiar with python to install python tools to run tests.

+1 to this. I'm also still strongly against the idea of having the primary way of having the user execute these be via the command line. It's not idiomatic and directly contradicts the idea of "copy-paste-runnable", which is one of the desirable goals that came out of the samples format working group.

If we have to include an interface via CLI, it should be additive and not have any effects on what the sample would look like without it.

averikitsch · 2019-10-23T18:16:14Z

+public class LanguageClassifyGcs {
+  // [START language_classify_gcs]
+  /*
+   * Please include the following imports to run this sample.


Having the imports listed again is very distracting.

kurtisvg · 2019-10-23T18:09:14Z

+   *
+   * @param gcsContentUri Google Cloud Storage URI where the file content is located. e.g.
+   *     gs://[Your Bucket]/[Path to File] The text file must include at least 20 words.
+   */


I don't think we should be using java-doc format for these. It's overly verbose, and no one is actually generating docs from these files so we aren't reaping any benefits.

kurtisvg · 2019-10-23T18:11:27Z

+   */
+  public static void sampleClassifyText(String gcsContentUri) {
+    try (LanguageServiceClient languageServiceClient = LanguageServiceClient.create()) {
+      // gcsContentUri = "gs://cloud-samples-data/language/classify-entertainment.txt";


Java-docs-samples is moving from commented out variables to declaration with an overloaded function instead. See GoogleCloudPlatform/java-docs-samples#1582 for an example.

Even as we move to over-loaded, there should still be a String at the front.
// String gcsContentUri ...

or var as its more modern. (but only in comments until we move to Java10+ as a base version

kurtisvg · 2019-10-23T18:12:59Z

+    try (LanguageServiceClient languageServiceClient = LanguageServiceClient.create()) {
+      // gcsContentUri = "gs://cloud-samples-data/language/classify-entertainment.txt";
+
+      // Available types: PLAIN_TEXT, HTML


+1 to doing this. Additionally, it would be great if a link to someplace in the docs that would give more context on what these values are (Particularly for the libraries that don't have an enum type, and instead just use magic string values).

kurtisvg · 2019-10-23T18:15:44Z

+public class LanguageClassifyGcs {
+  // [START language_classify_gcs]
+  /*
+   * Please include the following imports to run this sample.


Why are we including the imports as a comment instead of outside of the class?

kurtisvg · 2019-10-23T18:17:14Z

+import org.apache.commons.cli.Options;
+
+public class LanguageClassifyGcs {
+  // [START language_classify_gcs]


The current sample format has these include everything but the package name to make it easier for users to copy/paste.

kurtisvg · 2019-10-23T18:19:05Z

+   *     gs://[Your Bucket]/[Path to File] The text file must include at least 20 words.
+   */
+  public static void sampleClassifyText(String gcsContentUri) {
+    try (LanguageServiceClient languageServiceClient = LanguageServiceClient.create()) {


This should have a comment explaining why it's important to use the try-with-resources. Often users are going to be reusing a single client, so it's more likely they'll want to call close(). See the Client Initialization.

I.e. is this an initialized thing that I can use throughout the life of my app? is it thread-safe? (or do I need one for each thread (ie. request))? Is create an expensive (time consuming) operation?

All these things developers need guidance on.

kurtisvg · 2019-10-23T18:27:12Z

+              .setLanguage(language)
+              .build();
+      ClassifyTextRequest request = ClassifyTextRequest.newBuilder().setDocument(document).build();
+      ClassifyTextResponse response = languageServiceClient.classifyText(request);


Can we add some clarification here that the request isn't actually sent until classifyText is called? I feel like that's an easy mistake to make.

Also a nano style nit, but I would typically expect to see a newline between the request being built, the request being sent, and the response being processed. These feel like 3 very different actions, and seeing them together as single block makes it feel a little harder to parse (but maybe that's just me).

Some kind of comment about expected latencies here would be helpful.

kurtisvg · 2019-10-23T18:29:21Z

+        // is that this category represents the provided text.
+        System.out.printf("Confidence: %s\n", category.getConfidence());
+      }
+    } catch (Exception exception) {


This exception is way too broad - we should only be catching exceptions that we have to. For those exceptions, we should be showing the user how to correctly handle them.

@lesv is working on a proposal to clarify which errors/exceptions we should handle and what to do with them.

TL;DR; we should identify expected exceptions and provide either suggested mitigations (if any) or a clear explanation of what happened, if we aren't really doing anything useful, then we shouldn't be catching it.

kurtisvg · 2019-10-23T18:30:09Z

+// sample-metadata:
+//   title: Classify Content (GCS)
+//   description: Classifying Content in text file stored in Cloud Storage
+//   usage: gradle run -PmainClass=com.google.cloud.examples.language.v1.LanguageClassifyGcs [--args='[--gcs_content_uri "gs://cloud-samples-data/language/classify-entertainment.txt"]']


This should have example usage for maven and gradle.

/cc @lesv

Wow that's a horrifying command line. Great for cutting and pasting, but folks really aren't likely to get it right all that often. Is a gradle.build a part of the generated output?

I'm ok with just gradle, but there needs to be something, and it should be easier than the -PmainClass=... line you have.

vchudnov-g · 2019-10-23T19:42:30Z

@chingor13 @kurtisvg

Just to clarify a few points about sample-tester:

One of its use cases is that DPEs/API teams configuring samples can use it to make sure that the sample, which we tell users to compile and run, actually does behave as we tell them. Since it runs black-box tests (specified in a language-agnostic YAML file) for all languages, it's also available for repos to use in their CI if they choose. We had considered at one point generating language-specific test artifacts, as you suggest, but tabled that because it would mean replicating in each language functionality that is available in a language-agnostic tool....and dealing with the concomitant issues of making sure all implementations have feature parity as the tool evolves.
In terms of installing Python tools, we can easily package sample-tester to be a standalone executable that does not require a Python installation: you simply curl it and execute it. I've got that working on my machine and will formalize it as part of the sample-tester release process.

We are on the same page with the standalone samples being copy-paste-runnable, which is why if you compile these with javac they ought to just give you an executable and work. Is your comment alluding to the fact that working on the command line is itself not idiomatic in Java? If so, let's figure out a way to make execution idiomatic, though I would still hope to allow (but not require) each sample to be invoked via the CLI. I'm not an expert on the Java community, so your feedback is very valuable!

kurtisvg · 2019-10-23T22:52:14Z

We are on the same page with the standalone samples being copy-paste-runnable, which is why if you compile these with javac they ought to just give you an executable and work

When I say "copy-paste-runnable", I mean copying the snippet from a page in our documentation, pasting it into a local file, and making the smallest amount of changes to run it.

The vast majority (like 97%) of all Java users use either IntelliJ, Eclipse, or NetBeans as an IDE. Thus, I expect that most developers will likely first try to just copy and paste the snippet from the doc page they are looking at, and update the initial variables. They may also try to integrate it into existing code if they are prototyping a solution.

Our top priority with snippets should be getting users to understand how the call works, not just making sure they are able to execute it. My biggest issue with executing CLI is that historically it has added extra noise to the snippets that the user then has to shift through. Additionally, adding a CLI has often led to a README that only includes instructions for running passing arguments via the command-line, instead more general descriptions of what needs to be set or filled in to get the sample running. In an ideal world, the README would tell me which lines I need to change, and then let me to run the function however I prefer.

If we can figure out how to

Is your comment alluding to the fact that working on the command line is itself not idiomatic in Java?

Most users will expect either maven or gradle to handling building and verifying the code.

If I was trying to use this snippets to read from the API, my first instinct would be to copy-paste the code as I mentioned above. If I wasn't able to get it to run with minor debugging, my next step would be to clone the repo and run mvn verify (or the gradle equivalent) to trigger the tests and see if it passes. This is so standard in Java that all 3 of the big IDEs have a button on the toolbar by default to run the tests using a similar command.

If after messing around for a while and I happened to figure out that I needed a to download some random binary to run the tests, I would likely already have wasted a bunch of time and would be feeling frustrated that the owners felt the need to do something different.

We had considered at one point generating language-specific test artifacts, as you suggest, but tabled that because it would mean replicating in each language functionality that is available in a language-agnostic tool....and dealing with the concomitant issues of making sure all implementations have feature parity as the tool evolves.

I'm not sure if there are specific features you have in mind, but for java-docs-samples we've already established a pretty consistent pattern of capturing stdout, running a snippet's function, then checking that the correct output has occurred. This seems to be a similar surface to what the sample-tester is testing right now, but would allow for more idiomatic execution on per language basis.

vchudnov-g · 2019-10-24T00:57:12Z

Thanks for the info!

I mean copying the snippet from a page in our documentation, pasting it into a local file, and making the smallest amount of changes to run it.

Thanks for bringing that up. I should clarify that my earlier statement about copy-paste-runnable applies in the strictest sense only to the full source code of the sample: we mean for the samples to be ready to execute from the full file in the GitHub repo. So far, our strategy in all languages has been to place the region tags so that only the sample function (or just its body, depending on the language) gets displayed on cloudsite, while omitting the main() function and any boilerplate, such as imports, needed by main() (which is why we currently have the imports needed by the sample function commented within the region tags).

This strategy matches what is displayed by the current hand-written samples (cloudsite example) and reduces the boilerplate that people have to wade through when looking at the documentation on the website: we figured people would want to get right to the point. In that sense, the samples are not quite copy-paste-runnable from devsite and require some tweaking in most/all languages, which is why we have some of the generated comments (imports in Java, default variable assignments in languages where the region tags exclude the sample function signature). We are, of course, happy to change the location of the region tags if that's what DevRel needs. That's the purpose of this PR ;-)

If I was trying to use this snippets to read from the API, my first instinct would be to copy-paste the code as I mentioned above.

Fair enough. But if the sample did not include boilerplate or a main() would you then click on the "View on GitHub" button to get the full source code for the sample?
For instance, the current cloudsite sample points to the GitHub file which seems to expect CLI arguments.

If I wasn't able to get it to run with minor debugging, my next step would be to clone the repo and run mvn verify (or the gradle equivalent) to trigger the tests and see if it passes. This is so standard in Java that all 3 of the big IDEs have a button on the toolbar by default to run the tests using a similar command

Good to know. If you're already calling the repo, how would you feel if you could still call mvn verify and it used sample-tester under the hood (curling it and running it, invisibly to you)? I ask because writing language-idiomatic wrappers for sample-tester seems like an easy thing to do: it only needs to be done once, it doesn't duplicate existing functionality, and it allows the testing spec to evolve without sync/maintenance issues.

for java-docs-samples we've already established a pretty consistent pattern of capturing stdout, running a snippet's function, then checking that the correct output has occurred

That is mostly, but not entirely, what sample-tester is doing; it also has functionality to eg create UUIDs that will be needed in requests, read data from files, etc. Implementing it anew in Java would mean reading the language-agnostic testing YAML and acting on it, and implementing changes to the spec as we add features needed for further tests.

As an additional point, the sample-tester also tests not just that the sample function works, but that it is indeed copy-paste runnable (from source) as advertised.

nnegrey · 2019-10-24T15:20:03Z

@vchudnov-g just to note, those language samples you point to were written back in 2017. Our style rules have changed since then. They aren't the best representation of our current hand-written samples.

kurtisvg · 2019-10-24T16:53:15Z

@vchudnov-g just to note, those language samples you point to were written back in 2017. Our style rules have changed since then. They aren't the best representation of our current hand-written samples.

To further add to this, the DPEs had a working group last year that spent several months establishing ideal sample formats, creating canonical examples, and getting language specific stakeholders to sign off. Many different issues were discussed, debated, and compromised on during this process. It was also stated that these samples formats would be used as a target for these groups, but unfortunately this doesn't seem to be the case since many these same discussions are being revisited.

But if the sample did not include boilerplate or a main() would you then click on the "View on GitHub" button to get the full source code for the sample?

All three of the IDEs mentioned above allow you to simply click on a function and execute it without having to wrap it in a main().

Specifically with IntelliJ, I can just click on the function and be automatically brought to the UnitTest, which shows me how this function is intended to be called programmatically. I can then run that specific test or function as needed to verify that function is working like it's supposed to. I can even make small changes or tweaks to validate that something is working the way I think it is.

This is not an option if no tests exist and a separate binary has to be used to test them.

while omitting the main() function and any boilerplate, such as imports, needed by main() (which is why we currently have the imports needed by the sample function commented within the region tags)

A similar issue was raised in the working group. In short, we decided to include the class and imports to avoid pushing completing that boilerplate stuff on the user.

That is mostly, but not entirely, what sample-tester is doing; it also has functionality to eg create UUIDs that will be needed in requests, read data from files, etc.

Part of our current sample format dictates that we should minimize parameters to what's absolutely required for testing (which up to this point has been pretty much resource names). Not only was this done with intent to help cap the scope of difficulty to generating tests, but because we wanted to avoid making the assumption that users may already know the best way to read a file or some other task. Ideally, we should be showing users how to initialize these resources as part of it.

As an additional point, the sample-tester also tests not just that the sample function works, but that it is indeed copy-paste runnable (from source) as advertised.

This to me means that the snippet is executable, but not copy-paste-runnable. In summary, my concern as a whole is that our focus for the snippets seems to have shifted priority from "teach the user how to do something" to "get the user to do something". I think that making the snippet easy for the user to run is a big part of making it easy to learn from, but don't think that compiling and executing it is quite the same thing.

vchudnov-g · 2019-10-24T17:37:08Z

This discussion is very helpful! It sounds like there are at least the following issues:

location of the region tags: sounds like you want us to include all the top boilerplate and omit the main() from the cloudsite-displayed samples. That's not a problem and we're happy to defer to DevRel's decisions. IIUC the mental model you have is that users will copy-paste directly from cloudsite to an IDE, so we need to make sure that works without any hiccups and without relying on non-displayed code/configuration.
whether the sample files in GitHub should have a main() (outside the region tags): IMO it would be nice if they did, but it sounds like this use case lags far behind (1).
whether sample functions should have parameters

Part of our current sample format dictates that we should minimize parameters to what's absolutely required for testing (which up to this point has been pretty much resource names).

Sample parameters are set in the config, so there can be as may or as few are desired.

this done with intent to help cap the scope of difficulty to generating tests

All the sample parameters that are configured for samples can then be used for in configuring tests. Moreover, samples currently can be run directly (without sample-tester) with the default values of the parameters and they should just work---though granted, there may be some setup/teardown needed for methods that depend on or modify state (setup/teardown can be configured in sample-tester)

because we wanted to avoid making the assumption that users may already know the best way to read a file or some other task.

Under the current paradigm, tasks like reading from files are done in main() in order to pass the contents to the sample function. How to do these tasks is thus available to the user if they look at the GitHub file, though not if they only look at cloudsite. As per your feedback and this discussion, that paradigm may need to evolve so that we do display these tasks in cloudsite (ie put them within the region tags). I take it you're not concerned with the samples being too long there. (FWIW, our thinking had been to keep the cloudsite-displayed samples focused on the API-specific aspects, and the full sample on GitHub containing all the supplemental code that's not directly about making the the API call.)

kurtisvg · 2019-10-24T18:02:48Z

location of the region tags: sounds like you want us to include all the top boilerplate and omit the main() from the cloudsite-displayed samples. That's not a problem and we're happy to defer to DevRel's decisions. IIUC the mental model you have is that users will copy-paste directly from cloudsite to an IDE, so we need to make sure that works without any hiccups and without relying on non-displayed code/configuration.

Great 👍 Here is one of a current canonical samples (plus the update for the overloaded function) for reference. In particular, I think that the noargs function is critical if we decide not to generate tests, because it gives the user a foothold to configure and to run it instead.

whether the sample files in GitHub should have a main() (outside the region tags): IMO it would be nice if they did, but it sounds like this use case lags far behind (1).

I'm okay with it as long as it doesn't effect the sample that's down (like unnecessary imports). If so, I'd prefer we move the main() logic into a separate file to keep it separate.

FWIW, our thinking had been to keep the cloudsite-displayed samples focused on the API-specific aspect

This was originally our intent too, but IIRC, there was a UX study done in the original group (before I joined) that pointed to the vast majority of users discovering and using the code directly from the docs, and preferring not to visit GitHub.

lesv

Wow - Real progress - I love it.

I can live w/ just gradle, but I would like to see build.gradle files.
Don't love the command line, but can live with it.
don't see how your testing. Early on, we are often the canary in the coal mine for eng.
try / catch should do something, explain something, be specific, and offer mitigation suggestions.
com.example is better than com.google.cloud.examples for package names. See RFC 2606 & 6761

lesv · 2019-10-24T22:28:43Z

+// sample-metadata:
+//   title: Classify Content (GCS)
+//   description: Classifying Content in text file stored in Cloud Storage
+//   usage: gradle run -PmainClass=com.google.cloud.examples.language.v1.LanguageClassifyGcs [--args='[--gcs_content_uri "gs://cloud-samples-data/language/classify-entertainment.txt"]']


Wow that's a horrifying command line. Great for cutting and pasting, but folks really aren't likely to get it right all that often. Is a gradle.build a part of the generated output?

I'm ok with just gradle, but there needs to be something, and it should be easier than the -PmainClass=... line you have.

lesv · 2019-10-24T22:31:07Z

+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+// DO NOT EDIT! This is a generated sample ("Request",  "language_classify_gcs")


Perhaps we could use softer language than "DO NOT EDIT!", we expect our users to play with this and at times modify it to their needs. Just striking the DO NOT EDIT! is probably enough.

lesv · 2019-10-24T22:31:28Z

+import org.apache.commons.cli.Options;
+
+public class LanguageClassifyGcs {
+  // [START language_classify_gcs]


lesv · 2019-10-24T22:34:29Z

+  /*
+   * Please include the following imports to run this sample.
+   *
+   * import com.google.cloud.language.v1.ClassificationCategory;
+   * import com.google.cloud.language.v1.ClassifyTextRequest;
+   * import com.google.cloud.language.v1.ClassifyTextResponse;
+   * import com.google.cloud.language.v1.Document;
+   * import com.google.cloud.language.v1.LanguageServiceClient;
+   */


I'd rather this not be in the comment - why do this when it's already defined above?

lesv · 2019-10-24T22:38:32Z

+   *     gs://[Your Bucket]/[Path to File] The text file must include at least 20 words.
+   */
+  public static void sampleClassifyText(String gcsContentUri) {
+    try (LanguageServiceClient languageServiceClient = LanguageServiceClient.create()) {


I.e. is this an initialized thing that I can use throughout the life of my app? is it thread-safe? (or do I need one for each thread (ie. request))? Is create an expensive (time consuming) operation?

All these things developers need guidance on.

lesv · 2019-10-24T23:08:25Z

@@ -0,0 +1,60 @@
+---


It might be nice to explain this file somewhere. Like in a comment at the top.

lesv · 2019-10-24T23:10:24Z

-        artman_output_name='')
+        artman_output_name='',
+        include_samples=True)



Is this file checked in? Should it be?

lesv · 2019-10-24T23:14:24Z

+//   title: Transcribe Audio File using Long Running Operation (Local File) (LRO)
+//   description: Transcribe a long audio file using asynchronous speech recognition
+//   usage: gradle run -PmainClass=com.google.cloud.examples.speech.v1.SpeechTranscribeAsync [--args='[--local_file_path "resources/brooklyn_bridge.raw"]']


// This is a generated sample ("LongRunningRequestAsync", "speech_transcribe_async": // // Transcribe Audio File using Long Running Operation (Local File) (LRO) // // Transcribe a long audio file using asynchronous speech recognition // // usage: gradle run -PmainClass=com.example.speech.v1.SpeechTranscribeAsync [--args='[--local_file_path "resources/brooklyn_bridge.raw"]']

lesv · 2019-10-24T23:16:08Z

+      Path path = Paths.get(localFilePath);
+      byte[] data = Files.readAllBytes(path);
+      ByteString content = ByteString.copyFrom(data);


a comment & blank lines talking about reading from a local file (you'll need to adapt for your needs) would be good here.

lesv · 2019-10-24T23:18:49Z

+      Path path = Paths.get(localFilePath);
+      byte[] data = Files.readAllBytes(path);
+      ByteString content = ByteString.copyFrom(data);


bring out local system stuff.

yihanzhen · 2019-10-30T19:17:51Z

@lesv the gradle thing is a bug - there will (most likely) be a maven pom for the sample package, but I forgot to change from gradle to maven.

Added some GAPIC generated code samples for review

afdd328

googlebot added the cla: yes This human has signed the Contributor License Agreement. label Oct 23, 2019

chingor13 added the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label Oct 23, 2019

chingor13 requested review from kurtisvg, lesv and nnegrey October 23, 2019 17:54

averikitsch reviewed Oct 23, 2019

View reviewed changes

kurtisvg suggested changes Oct 23, 2019

View reviewed changes

lesv suggested changes Oct 24, 2019

View reviewed changes

yihanzhen mentioned this pull request Nov 5, 2019

Java samplegen: replace comment-out default lines with an overload of the sample method googleapis/gapic-generator#3019

Merged

yihanzhen closed this in googleapis/gapic-generator#3019 Nov 6, 2019

beccasaurus mentioned this pull request Dec 17, 2019

[Syntax Review] Additional types of SampleGen code samples #7051

Closed

Conversation

beccasaurus commented Oct 23, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Language experts, please TAL! 🧐

Uh oh!

chingor13 commented Oct 23, 2019

Uh oh!

kurtisvg commented Oct 23, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vchudnov-g commented Oct 23, 2019

Uh oh!

kurtisvg commented Oct 23, 2019

Uh oh!

vchudnov-g commented Oct 24, 2019

Uh oh!

nnegrey commented Oct 24, 2019

Uh oh!

kurtisvg commented Oct 24, 2019

Uh oh!

vchudnov-g commented Oct 24, 2019

Uh oh!

kurtisvg commented Oct 24, 2019

Uh oh!

lesv left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

beccasaurus commented Oct 23, 2019 •

edited

Loading