Scoverage work - add scoverage to OSS pants #8064

sammy-1234 · 2019-07-17T16:48:23Z

Problem

This PR is in response to issue #8026.

Solution

It attempts to add scoverge by creating a new subsystem scoverage_platform.py and modifies the scala_library target according to the subsystem in case scoverage is enabled.
Furthermore, it adds a scoverage.py runtime support required for report generation.

Result

Users can run scoverage by running the following command:
./pants --scoverage-enable-scoverage=True test path/to/<scala-target>

Tests

I have added tests for the new subsystem in test_scoverage_platform.py. Run them as follows:
./pants test tests/python/pants_test/backend/jvm/subsystems:test_scoverage_platform

Files changed

Please take a look at my comment below for a list of all the modified files.

Eric-Arellano

Only looked at where you can use Python 3. Also did this from my phone so please message me on Slack if any of this is confusing.

Eric-Arellano · 2019-07-17T16:55:45Z

src/python/pants/backend/jvm/subsystems/scala_coverage_platform.py

+# Copyright 2015 Pants project contributors (see CONTRIBUTORS.md).
+# Licensed under the Apache License, Version 2.0 (see LICENSE).
+
+from __future__ import absolute_import, division, print_function, unicode_literals


Don’t need this anymore.

Eric-Arellano · 2019-07-17T16:55:56Z

src/python/pants/backend/jvm/subsystems/scala_coverage_platform.py

@@ -0,0 +1,94 @@
+# coding=utf-8


Don’t need this because we don’t care about Python 2 support.

src/python/pants/backend/jvm/subsystems/scala_coverage_platform.py

Eric-Arellano · 2019-07-17T16:58:17Z

src/python/pants/backend/jvm/subsystems/scala_coverage_platform.py

+  @property
+  def injectables_spec_mapping(self):
+    return {
+      'scoverage': ['{}'.format(self.get_options().scoverage_target_path)],


Can use an f string

`f”{self.get_options...}”

Eric-Arellano · 2019-07-17T16:59:53Z

src/python/pants/backend/jvm/subsystems/scala_coverage_platform.py

+    """
+    Adds 'scoverage' to scalac_plugins in case scoverage is enabled for that [target].
+    :return: modified scalac_plugins
+    :rtype: list of strings


Rather than putting this in docstring, can use a type hint. On line 40 put -> List[str] between the param list and colon. See strutil.py for an example.

You’ll also need to add from typing import List

Eric-Arellano · 2019-07-17T18:05:13Z

src/python/pants/backend/jvm/subsystems/scala_coverage_platform.py

+    """
+    Adds 'scoverage' to scalac_plugins_args in case scoverage is enabled for that [target].
+    :return: modified scalac_plugins_args
+    :rtype: map from string to list of strings.


Return type is Dict[str, List[str]]

Eric-Arellano · 2019-07-17T18:05:47Z

src/python/pants/backend/jvm/subsystems/scala_coverage_platform.py

+    scalac_plugin_args = target.payload.scalac_plugin_args
+    if scalac_plugin_args:
+      scalac_plugin_args.update(
+        {"scoverage": ["writeToClasspath:true", "dataDir:{}".format(target.identifier)]})


F strings here and a few lines below

sammy-1234 · 2019-07-18T06:19:55Z

Thanks @Eric-Arellano 💯 for python3 help.

The latest commit removes scoverage target from BUILD.tools and creates an "injectable" for the same in ScalaCoveragePlatform subsystem.

…m.py Co-Authored-By: Eric Arellano <[email protected]>

stuhood

Thanks! Looks like a great start.

I'd like to see (at least) a draft of the second half of the change before we land this though.

stuhood · 2019-07-19T01:02:52Z

3rdparty/jvm/org/scoverage/BUILD

@@ -0,0 +1,3 @@
+target(


I believe that this target can be removed now.

stuhood · 2019-07-19T01:05:06Z

pants.ini

@@ -220,6 +220,13 @@ no_warning_args: [
    '-S-nowarn',
  ]

+compiler_option_sets_enabled_args: {
+    # Needed to make scoverage CodeGrid highlighting work
+    'scoverage': [


Rather than having to manually include this, we should automatically include it when scoverage is enabled. Probably somewhere around here?

pants/src/python/pants/backend/jvm/tasks/jvm_compile/zinc/zinc_compile.py

Lines 324 to 325 in 4435b8e

compiler_option_sets_args = self.get_merged_args_for_compiler_option_sets(compiler_option_sets)

zinc_args.extend(compiler_option_sets_args)

stuhood · 2019-07-19T01:06:10Z

src/python/pants/backend/jvm/subsystems/scala_coverage_platform.py

+blacklist_file = 'new_blacklist_scoverage'
+
+
+class ScalaCoveragePlatform(InjectablesMixin, Subsystem):


Naming nit: rather than calling this ScalaCoverage, we should probably call it Scoverage, as that seems to be the project's actual name.

stuhood · 2019-07-19T01:06:19Z

src/python/pants/backend/jvm/subsystems/scala_coverage_platform.py

+class ScalaCoveragePlatform(InjectablesMixin, Subsystem):
+  """The scala coverage platform."""
+
+  options_scope = 'scala-coverage'


ditto naming.

stuhood · 2019-07-19T01:07:43Z

src/python/pants/backend/jvm/subsystems/scala_coverage_platform.py

+    register('--enable-scoverage',
+      default=False,
+      type=bool,
+      help='Specifies whether to generate scoverage reports for scala test targets.'


This should be consumed from this subsystem instead:

pants/src/python/pants/backend/jvm/tasks/coverage/manager.py

Lines 73 to 74 in 4435b8e

register('--coverage-open', type=bool, fingerprint=True,

help='Open the generated HTML coverage report in a browser. Implies --coverage.')

If I understand correctly, manager.py is initialized by junit_run.py at runtime only. However, I also need this option (enable-scoverage) at compile time to determine whether to instrument the targets or not. That is why I kept it here. Am I thinking about this right?

I think I see what is happening here: Even though the CodeCoverage subsystem gets initialized, none of its options are registered. This is because the subsystem is using def register_junit_options(register, register_jvm_tool): instead of def register_options(cls, register):

while the options under the latter (register_options) gets automatically registered, the former (register_junit_options) requires to be called by some other code, e.g, this is how it is called in junit_run.py:

pants/src/python/pants/backend/jvm/tasks/junit_run.py

Lines 99 to 102 in 35be511

# TODO(jtrobec): Remove direct register when coverage steps are moved to their own subsystem.

CodeCoverage.register_junit_options(register, cls.register_jvm_tool)

And when it is called by some other class,( junit in this example), the scope of that class gets prepended to the scope of coverage, thus registering the options as --test-junit-coverage

Consequently, setting --scoverage-coverage=True (scope at compile time) does not automatically set --test-junit-coverage=True(scope at runtime) (and vice-versa)

stuhood · 2019-07-19T01:10:58Z

src/python/pants/backend/jvm/targets/scala_library.py

+    :return: See constructor.
+    :rtype: list of strings.
+    """
+    if self._scoverage:


Rather than fully overriding the set of plugins, this should merge both lists. Ditto the plugin args and options.

the function get_scalac_plugins in scoverage_platform.py does that. It merges the list if initially non empty (otherwise creates a new one) Similarly, for plugin args and options, the merging happens inside the scoverage subsystem.

Ok. That's a bit confusing, because if someone were to change line 102 here, they would need to remember to change the subsystem code as well. IMO, the merging code should live in here to avoid that issue.

sammy-1234 · 2019-07-20T21:25:46Z

Scoverage (2/2) - Adding report generator
This 2nd PR is a draft of how report generator would look in pants.

Files Changes

backend/jvm/tasks/coverage/manager.py: Adding support for scoverage

New Files Added

backend/jvm/tasks/coverage/scoverage.py: scoverage runtime subsystem
src/scala/org/pantsbuild/scoverage/report/ScoverageRep.scala: scoverage report generator
src/scala/org/pantsbuild/scoverage/report/Settings.scala: scoverage report generator settings

stuhood

Thanks! This looks really good.

As mentioned below: please break out a separate PR for the scoverage report generation JVM code. We'll have to land that first.

stuhood · 2019-07-22T18:31:45Z

src/python/pants/backend/jvm/targets/scala_library.py

+    :return: See constructor.
+    :rtype: list of strings.
+    """
+    if self._scoverage:


Ok. That's a bit confusing, because if someone were to change line 102 here, they would need to remember to change the subsystem code as well. IMO, the merging code should live in here to avoid that issue.

src/python/pants/backend/jvm/subsystems/scoverage_platform.py

stuhood · 2019-07-22T18:41:01Z

src/python/pants/backend/jvm/tasks/coverage/scoverage.py

+    if self._settings.coverage_open:
+      return os.path.join(base_report_dir, 'html', 'index.html')
+
+  def _execute_scoverage_report_gen(self, measurements_dir, report_dir, target_filter):


Another PR will need to be broken out to land the report generation JVM code so that we can publish it to maven central and then consume it here as a register_jvm_tool.

BUILD.tools

nsaechao · 2019-07-22T22:44:24Z

src/python/pants/backend/jvm/subsystems/BUILD

+  dependencies = [
+    'src/python/pants/option',
+    'src/python/pants/subsystem',
+     'src/python/pants/java/jar',


nit: extra space?

src/python/pants/backend/jvm/subsystems/scoverage_platform.py

nsaechao · 2019-07-22T22:56:26Z

src/python/pants/backend/jvm/subsystems/scoverage_platform.py

+      compiler_option_sets = [SCOVERAGE]
+    return tuple(compiler_option_sets)
+
+  def is_blacklisted(self, target) -> bool:


The blacklist should be cached in-memory, currently the file is read per target.

If you switch this to a list option (as above) that will be handled automatically.

src/python/pants/backend/jvm/tasks/coverage/scoverage.py

nsaechao · 2019-07-22T23:20:15Z

src/python/pants/backend/jvm/tasks/coverage/scoverage.py

+      f'"{report_dir}"]',
+    ]
+
+    # if target_filter:


dead code? can be removed

src/scala/org/pantsbuild/scoverage/report/ScoverageReport.scala

…blacklisted files

…nto scoverageWork

sammy-1234 · 2019-07-24T22:15:08Z

Scoverage Outline

Major files added

scoverage_platform.py: Scoverage subsystem
scoverage.py: Scoverage runtime environment (similar to Jacoco and Cobertura)
test_scoverage_platform.py: tests for scoverage

Major files changed

scala_library.py: modifications to scala library in case scoverage is enabled
manager.py: modifying the coverage manager to choose scoverage in case it is enabled
register.py: registering the subsystem
zinc.py: Adding compiler_options_sets_args for scoverage to work.

Other changes

Adding required build files
Changing existsing tests to add Scoverage dependency in order to pass unit tests,

…error

stuhood

Thanks!

stuhood · 2019-07-25T16:25:32Z

src/python/pants/backend/jvm/subsystems/scoverage_platform.py

+      default=False,
+      type=bool,
+      help='Specifies whether to generate scoverage reports for scala test targets.'
+           'Default value is False. If True,'


These lines need spaces in them in order to avoid running together.

stuhood · 2019-07-25T16:28:01Z

src/python/pants/backend/jvm/subsystems/scoverage_platform.py

+           'Default value is False. If True,'
+           'implies --test-junit-coverage-processor=scoverage.')
+
+    register('--blacklist-file',


Pants supports @file options natively, so you should make this option a list option:

register('--blacklist-targets', type=list, member_type=target_option, .. )

And then someone who wants to put the list of targets in a file (in python list syntax) can pass --blacklist-targets=@some-blacklist-file.

You could also pass in a (string) list of regexes to match against target names.

stuhood · 2019-07-25T16:29:28Z

src/python/pants/backend/jvm/subsystems/scoverage_platform.py

+      compiler_option_sets = [SCOVERAGE]
+    return tuple(compiler_option_sets)
+
+  def is_blacklisted(self, target) -> bool:


If you switch this to a list option (as above) that will be handled automatically.

stuhood · 2019-07-25T16:30:04Z

src/python/pants/backend/jvm/subsystems/scoverage_platform.py

+      return False
+
+    if target.address.spec in self._blacklist_file_contents:
+      logger.warning(f"{target.address.spec} found in blacklist, not instrumented.")


This should probably be at debug or info rather than warn.

stuhood · 2019-07-25T16:30:48Z

src/python/pants/backend/jvm/targets/scala_library.py

@@ -47,6 +51,8 @@ def __init__(self, java_sources=None, payload=None, **kwargs):
    })
    super().__init__(payload=payload, **kwargs)

+    self._scoverage_instance = ScoveragePlatform.global_instance()


The target itself doesn't need to hold a reference to _scoverage_instance: can get it lazily from the singleton.

stuhood · 2019-07-25T16:40:14Z

src/python/pants/backend/jvm/tasks/coverage/scoverage.py

+        ]
+      )
+
+      register('--target-filters', type=list, default=[],


This feels redundant with the ScoveragePlatform.blacklist option. Do we need both?

This target filtering is for report generation, i.e helps in generating reports only for the specified targets. The blacklist option is more for overcoming the issue that if some targets cannot be instrumented at all (due to exceeding JVM code size limits), they should be passed into the blacklist.

this option helps in preventing the need to compile again if the report is needed only for a handful of targets, i.e, say I compiled 500 targets and got the report generated. But now I need a report for all targets containing the class "merlin"(~ 10 targets), or all targets in package "report", then I can get those reports without needing to compile everything again.

Hm. But in reality, are people going to know that? From a user's perspective, having just one option would likely be easier to understand.

Agreed. People just need to use this target-filter option most of the time. In case they encounter this error of JVM code size limits, they can overcome that with the help of blacklist option as well. More like a Troubleshooting mechanism. I'll make sure to document this.

stuhood · 2019-07-25T16:42:00Z

src/python/pants/backend/jvm/tasks/coverage/scoverage.py

+      def slf4j_jar(name):
+        return JarDependency(org='org.slf4j', name=name, rev='1.7.5')
+
+      def scopt_jar():


This is only used one place, so maybe just inline it.

stuhood · 2019-07-25T16:43:14Z

src/python/pants/backend/jvm/tasks/coverage/scoverage.py

+  def _iter_datafiles(self, output_dir):
+    for root, _, files in safe_walk(output_dir):
+      for f in files:
+        if f.startswith("scoverage"):


Should move these comments into python method docs:

def _iter_datafiles(self, output_dir): """This is a docstring for a method. With some other stuff in it. """ ..

src/python/pants/backend/jvm/tasks/coverage/scoverage.py

stuhood · 2019-07-25T16:46:39Z

src/python/pants/backend/jvm/tasks/jvm_compile/zinc/zinc_compile.py

@@ -322,6 +322,11 @@ def relative_to_exec_root(path):
    zinc_args.append('-transactional')

    compiler_option_sets_args = self.get_merged_args_for_compiler_option_sets(compiler_option_sets)
+
+    # Needed to make scoverage CodeGrid highlighting work


This should be incorporated in ScalaLibrary rather than here.

How can I set compiler_option_set_enabled_args in ScalaLibrary. I looked at the compiler_option_sets_mixin.py here : https://github.com/pantsbuild/pants/blob/14e0bd6b27910310e225ff9586c2410459153fd9/src/python/pants/option/compiler_option_sets_mixin.py But there is no API for adding extra arguments. That is I why added it manually here.

Ok, fine with this as is I guess.

nsaechao · 2019-07-25T19:03:01Z

src/python/pants/backend/jvm/subsystems/scoverage_platform.py

+      help='Path to files containing targets not to be instrumented.')
+
+    register('--scoverage-target-path',
+      default='//:scoverage',


It looks like this target is bootstrapped by default. If a user overrides this, the default //:scoverage target is still injected into the build graph.

nsaechao

Thanks Sameer for doing this awesome work, based off my previous review and assuming you address Stu's concerns/changes. Everything looks good to me.

stuhood

Looks great! Thanks.

stuhood · 2019-07-26T18:24:39Z

src/python/pants/backend/jvm/targets/scala_library.py

+      else:
+        compiler_option_sets = [SCOVERAGE]
+
+      super().__init__(payload=payload,


Having two super calls is pretty weird, and in general this has resulted in a lot of nesting.

An alternative might be to do something like:

scalac_plugins = ... or [] scalac_plugin_args = ... or [] compiler_option_sets = ... or [] if ScoveragePlatform.global_instance().get_options().enable_scoverage: scalac_plugins.append(..) scalac_plugin_args.update(..) compiler_option_sets.update(..) super(...)

stuhood · 2019-07-26T18:26:50Z

src/python/pants/backend/jvm/tasks/coverage/scoverage.py

+        ]
+      )
+
+      register('--target-filters', type=list, default=[],


Hm. But in reality, are people going to know that? From a user's perspective, having just one option would likely be easier to understand.

src/python/pants/backend/jvm/tasks/coverage/scoverage.py

stuhood · 2019-07-26T18:29:49Z

src/python/pants/backend/jvm/tasks/jvm_compile/zinc/zinc_compile.py

@@ -322,6 +322,11 @@ def relative_to_exec_root(path):
    zinc_args.append('-transactional')

    compiler_option_sets_args = self.get_merged_args_for_compiler_option_sets(compiler_option_sets)
+
+    # Needed to make scoverage CodeGrid highlighting work


Ok, fine with this as is I guess.

stuhood · 2019-07-27T01:50:33Z

@sammy-1234: re: the lint errors: are you using the commit hooks? https://www.pantsbuild.org/howto_contribute.html#getting-pants-source-code

sammy-1234 · 2019-07-27T03:12:40Z

Thanks for that. But even then, it is still giving me this weird error:
[INFO] pexrc interpreters requested and found, but other paths were also specified, so interpreters may not be restricted to the pexrc ones. Full search path is: /opt/ee/python/2.7/bin/python2.7:/opt/ee/python/3.6/bin/python3.6:/usr/local/bin/python3.7:/Users/sameera/Desktop/forked_pants/pants/build-support/pants_dev_deps.py37.venv/bin:/opt/twitter_mde/bin:/opt/twitter_mde/homebrew_minimal/mde_bin:/usr/local/mysql/bin:/Users/sameera/bin:/usr/local/bin:/usr/bin:/bin:/usr/sbin:/sbin:/usr/local/munki:/opt/twitter_mde/data/node/bin:/Users/sameera/.npm-global/bin

and then failing with:

Failed to execute PEX file, missing macosx_10_14_x86_64-cp-27-cp27m compatible dependencies for: mypy

Any clue how I can resolve this? Thanks :)

stuhood · 2019-07-27T15:19:25Z

@sammy-1234 : If you have an /etc/pexrc file, you should: sudo mv /etc/pexrc /etc/pexrc.old, run clean-all, and then the hooks will work. This is an unfortunate sideeffect of some of the setup on our laptops.

sammy-1234 · 2019-07-27T17:01:22Z

Thanks @stuhood . The hooks pass now 💯

adding scoverage 1/2

3ef8021

Eric-Arellano reviewed Jul 17, 2019

View reviewed changes

Sameer Arora added 3 commits July 17, 2019 23:40

adding scoverage 1/2

705a12b

adding scoverage 1/2

5ea8fc5

removing scoverage target from BUILD.tools

fc48b64

sammy-1234 force-pushed the scoverageWork branch from d11fd6c to fc48b64 Compare July 18, 2019 06:41

sammy-1234 and others added 2 commits July 17, 2019 23:44

Update src/python/pants/backend/jvm/subsystems/scala_coverage_platfor…

21a5dd4

…m.py Co-Authored-By: Eric Arellano <[email protected]>

removing futures

2429357

stuhood reviewed Jul 19, 2019

View reviewed changes

Sameer Arora added 3 commits July 19, 2019 00:23

changing name to scoverage_platform

f432d7b

adding scoverage dependency for unit tests to pass

c7f560a

Scoverage (2/2) - adding report generator

c427e21

changing report generator's name

324f59d

stuhood reviewed Jul 22, 2019

View reviewed changes

sammy-1234 changed the title ~~Scoverage work (1/2) - add scoverage to OSS pants~~ Scoverage work - add scoverage to OSS pants Jul 22, 2019

nsaechao reviewed Jul 22, 2019

View reviewed changes

Sameer Arora and others added 12 commits July 23, 2019 09:50

scoverage report generator

1a7e8fc

adding scoverage report gen

739032a

testing with new report gen local publish

fc4ed94

changing scoverage to work with new reportGen

3212da7

relocatiing modifications to scala library

f9561dc

playing with new report gen

3d913e3

adding scoverage runtime environment along with target filtering and …

3209a9f

…blacklisted files

removing report generator from here

3cd1b8b

Merge branch 'master' into scoverageWork

3099ac7

resolving unwanted file changes

66ace61

Merge branch 'scoverageWork' of https://github.com/sammy-1234/pants i…

3ff84cc

…nto scoverageWork

minor changes to alignment

a47ef75

Sameer Arora added 5 commits July 24, 2019 15:26

minor changes to alignment

7bacd7b

minor changes to alignment

d9a94e1

adding log statements

37295d6

improving test file for blacklisted targets

f390ee4

adding scoverage dependency to test_rsc_compile to resolve unit test …

b15f45d

…error

stuhood reviewed Jul 25, 2019

View reviewed changes

nsaechao reviewed Jul 25, 2019

View reviewed changes

Sameer Arora added 5 commits July 25, 2019 16:16

adding extra args in constructor rather than as a property

ae22fed

removing target's scoverage instance

e5b0a56

fixing blacklisted target specification

12430dd

cleaning checking of blacklisted targets; fixing scoverage test

04065ac

removing option; removing target spec as an option

e13edd8

nsaechao approved these changes Jul 26, 2019

View reviewed changes

stuhood approved these changes Jul 26, 2019

View reviewed changes

Sameer Arora added 3 commits July 26, 2019 12:39

init change in scala_library; linting

4e1b5c8

fixing lint again

02435cc

trying to fix lint in test_scoverage_platform

ed7d4b0

changing args indentation in scoverage.py

7acd82c

running git hooks successfully

d6a2dbc

stuhood merged commit 23a8a2b into pantsbuild:master Jul 27, 2019

sammy-1234 deleted the scoverageWork branch July 27, 2019 20:57

Romainj1 mentioned this pull request Aug 26, 2019

Create a scoverage plugin for pants #7540

Closed

ckipp01 mentioned this pull request Sep 27, 2021

Create a scoverage plugin for pants scoverage/scalac-scoverage-plugin#249

Closed

	compiler_option_sets_args = self.get_merged_args_for_compiler_option_sets(compiler_option_sets)
	zinc_args.extend(compiler_option_sets_args)

		blacklist_file = 'new_blacklist_scoverage'


		class ScalaCoveragePlatform(InjectablesMixin, Subsystem):

	register('--coverage-open', type=bool, fingerprint=True,
	help='Open the generated HTML coverage report in a browser. Implies --coverage.')


	# TODO(jtrobec): Remove direct register when coverage steps are moved to their own subsystem.
	CodeCoverage.register_junit_options(register, cls.register_jvm_tool)

Scoverage work - add scoverage to OSS pants #8064

Scoverage work - add scoverage to OSS pants #8064

Conversation

sammy-1234 commented Jul 17, 2019 • edited Loading

Problem

Solution

Result

Tests

Files changed

Eric-Arellano left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sammy-1234 commented Jul 18, 2019 • edited Loading

stuhood left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sammy-1234 Jul 19, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sammy-1234 commented Jul 20, 2019 • edited Loading

Files Changes

New Files Added

stuhood left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sammy-1234 commented Jul 24, 2019 • edited Loading

Major files added

Major files changed

Other changes

stuhood left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sammy-1234 Jul 25, 2019 • edited Loading

Choose a reason for hiding this comment

sammy-1234 Jul 26, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sammy-1234 Jul 26, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sammy-1234 Jul 25, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nsaechao left a comment

Choose a reason for hiding this comment

stuhood left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

stuhood commented Jul 27, 2019

sammy-1234 commented Jul 27, 2019

stuhood commented Jul 27, 2019 • edited Loading

sammy-1234 commented Jul 27, 2019

sammy-1234 commented Jul 17, 2019 •

edited

Loading

sammy-1234 commented Jul 18, 2019 •

edited

Loading

sammy-1234 Jul 19, 2019 •

edited

Loading

sammy-1234 commented Jul 20, 2019 •

edited

Loading

sammy-1234 commented Jul 24, 2019 •

edited

Loading

sammy-1234 Jul 25, 2019 •

edited

Loading

sammy-1234 Jul 26, 2019 •

edited

Loading

sammy-1234 Jul 26, 2019 •

edited

Loading

sammy-1234 Jul 25, 2019 •

edited

Loading

stuhood commented Jul 27, 2019 •

edited

Loading