moved waiter from diagnostics.dll to features.dll where all interface… by heejaechang · Pull Request #24120 · dotnet/roslyn

heejaechang · 2018-01-09T11:23:42Z

…s are defined.

Customer scenario

This doesn't affect customer experience. this is purely for testing.

Bugs this fixes

https://devdiv.visualstudio.com/DevDiv/_workitems/edit/549239

Workarounds, if any

install our Roslyn.Test.Setup.vsix to machines that want to test our asynchronous features.

Risk

This only affects Testing, so there shouldn't be user facing risk.

Performance impact

There should be no perf impact due to this change to users. only affects testing.

Is this a regression from a previous update?

N/A

Root cause analysis

As all other Roslyn design, we separated out test related component from product component, and used vsix and MEF to inject those when it is needed (test env). it gave us clean separation. but gave us yet one more thing to setup when running test.

it was fine for our own test env, but when it meets with other components in VS and when other team needed to run some of their test with our test hook, it became yet one more point that can fail for test setup.

so, we now put those hooks in Roslyn dlls itself which can be enabled by env variable.

How was the bug found?

during testing.

…s are defined.

heejaechang · 2018-01-09T12:06:11Z

src/Features/Core/Portable/Shared/TestHooks/Waiters/EditorAdornmentWaiter.cs

 namespace Roslyn.Hosting.Diagnostics.Waiters
 {
-    internal abstract class EditorAdornmentWaiter : AsynchronousOperationListener
+    internal abstract class EditorAdornmentWaiter


left these files since I wasn't sure what this is trying to accomplish. since it continues after task being completed. the task itself shouldn't rely on UI thread being ran. so I wasn't sure what this Dispatcher is trying to do. if there is some work scheduled to UI thread, the original task should wait for it so, the chained dispatcher doesn't have any impact?

…o us.

…est. that dll is not in probing path.

heejaechang · 2018-01-11T00:50:21Z

@dotnet/roslyn-infrastructure turns out the failure in integration test has nothing to do with my change. it was just a bug in integration test. I am not sure how it was ever worked...?
...
never mind, found out why it used to work.

…e event queue get proper async listener

heejaechang · 2018-01-11T11:43:00Z

@jasonmalinowski @dotnet/roslyn-ide @dotnet/roslyn-analysis can you take a look?

I simplified our listener/waiter thing. and put everything basically in one central end point (AsynchronousOperationListenerProvider).

it no longer uses MEF for each listener. only MEF is the provider itself. and it is MEF because I am using MEF for isolation in unit test.

all other changes are just replacing one to the other. actual change is in the provider.

heejaechang · 2018-01-11T11:43:39Z

@brettfo so, you added a way to call our Waiter in apex code? @ivanbasov can you point me how I can update Apex code?

heejaechang · 2018-01-11T11:45:25Z

src/Features/Core/Portable/Shared/TestHooks/IAsynchronousOperationListenerProvider.cs

+        {
+            get
+            {
+                if (!s_enabled.HasValue)


didn't use lock for this and the one below. these are not expected to called multiple time. but only once for 1 VS session.

brettfo · 2018-01-11T18:39:40Z

@heejaechang: The change to consume this waiter in Apex is already in the internal code. I'll email you the details.

jasonmalinowski

My biggest complaint is we didn't do this sooner! Thanks for cleaning this one up.

A few nitpick questions, and I'd like a few bits of strangeness around interfaces/concrete types fixed up where we have some casts that make you go "hmm?".

jasonmalinowski · 2018-01-11T18:49:25Z

src/EditorFeatures/CSharpTest/Workspaces/WorkspaceTests.cs

-                .GetExports<IAsynchronousOperationListener, FeatureMetadata>()
-                .First(l => l.Metadata.FeatureName == FeatureAttribute.Workspace).Value as IAsynchronousOperationWaiter;
+                                           .GetExportedValue<IAsynchronousOperationListenerProvider>()
+                                           .GetListener(FeatureAttribute.Workspace) as IAsynchronousOperationWaiter;


I'm guessing the "as" is no longer necessary here? (and I'm not sure why we'd ever use as since it'll null ref if it's wrong...)

yep. it wasn't need to. I was just lazy :) changing to as ... was quicker than move to front and put (...) :) but sure, I can change it to cast.

jasonmalinowski · 2018-01-11T18:53:53Z

src/EditorFeatures/Test/RenameTracking/RenameTrackingTestState.cs

        {
-            var waiters = Workspace.ExportProvider.GetExportedValues<IAsynchronousOperationWaiter>();
-            await waiters.WaitAllAsync();
+            var provider = Workspace.ExportProvider.GetExportedValue<IAsynchronousOperationListenerProvider>() as AsynchronousOperationListenerProvider;


Seems fishy here to be grabbing by the interface and then an "as" cast when (a) it should be a hard cast and (b) why can't we just export the concrete type directly?

sure. I can expose concrete type as MFE as well. I believe same type expose 2 MEF types, it still uses same instance right?

GetWaiter doesn't exist in interface since waiter is not supposed to be used in product. and the interface is for the product code.

jasonmalinowski · 2018-01-11T18:54:27Z

src/EditorFeatures/Test/SolutionCrawler/WorkCoordinatorTests.cs

-            internal WorkspaceWaiter()
-            {
-            }
+            return (AsynchronousOperationListenerProvider)provider.GetExportedValue<IAsynchronousOperationListenerProvider>();


Why the concrete type?

waiter only exist in concrete type. interface only provide a way to get listener.

jasonmalinowski · 2018-01-11T18:57:28Z

src/EditorFeatures/Test2/Classification/ClassificationTests.vb

-                    SpecializedCollections.SingletonEnumerable(
-                        New Lazy(Of IAsynchronousOperationListener, FeatureMetadata)(
-                            Function() waiter, New FeatureMetadata(New Dictionary(Of String, Object)() From {{"FeatureName", FeatureAttribute.Classification}}))))
+                    listenerProvider)


Well that's a lot simpler. :-)

jasonmalinowski · 2018-01-11T18:58:42Z

src/EditorFeatures/Test2/NavigationBar/NavigationBarPresenterTests.vb


-                Dim waiters = workspace.ExportProvider.GetExportedValues(Of IAsynchronousOperationWaiter)
-                Await waiters.WaitAllAsync()
+                Dim provider = DirectCast(workspace.ExportProvider.GetExportedValue(Of IAsynchronousOperationListenerProvider), AsynchronousOperationListenerProvider)


As earlier comment: odd to have both interface and concrete type here...

jasonmalinowski · 2018-01-11T19:20:42Z

src/Features/Core/Portable/Shared/TestHooks/IAsynchronousOperationListenerProvider.cs

+
+        private IEnumerable<IAsynchronousOperationWaiter> GetCandidateWaiters(string[] featureNames)
+        {
+            if (featureNames == null || featureNames.Length == 0)


I guess I'd consider null OK but an empty list to be an error...

method(params string[] xxx) gives empty array, so need to check empty array. this particular method doesn't use params, but most of caller to this method does and i didn't want to put if (xx.length == 0)? return null to all those caller :)

jasonmalinowski · 2018-01-11T19:21:50Z

src/Test/Diagnostics/TestingOnly_WaitingService.cs

 namespace Roslyn.Hosting.Diagnostics.Waiters
 {
    [Export, Shared]
    public class TestingOnly_WaitingService


Do we even need this type anymore? It seems like the other type does all the same stuff?

this is public. this is for ones that doesn't have access to roslyn, but want to use waiter as long as they can somehow get to this Roslyn.xxx.Disagnostics.dll

also, our integration test uses it. so.. I left it.

jasonmalinowski · 2018-01-11T19:22:32Z

src/Test/Diagnostics/Waiters/EditorAdornmentWaiter.cs

-{
-    internal abstract class EditorAdornmentWaiter : AsynchronousOperationListener
-    {
-        public override Task CreateWaitTask()


I'm horrified this was virtual and had a special case like this. Do we still need this logic? (I hope not...?)

the way it does was incorrect I believe. since it basically wanted to call DoEvent for other waiter (not this waiter). now I explicitly call Doevent while waiting rather than some random waiter calling it hoping that is enough for other waiter stuck waiting for UI context to proceed.

jasonmalinowski · 2018-01-11T19:23:36Z

src/VisualStudio/Core/Test/DebuggerIntelliSense/CompletionWaiter.vb

@@ -1,16 +0,0 @@
-' Copyright (c) Microsoft.  All Rights Reserved.  Licensed under the Apache License, Version 2.0.  See License.txt in the project root for license information.
-
-#If False Then


jasonmalinowski · 2018-01-11T19:24:50Z

src/VisualStudio/IntegrationTest/TestUtilities/InProcess/VisualStudioWorkspace_InProc.cs


        private VisualStudioWorkspace_InProc()
        {
+            // we need to enable waiting service before we create workspace


I think you'd want to move this to somewhere earlier when we are actually setting up the cross-process communication. This might be too late at this point.

I confirmed this is early enough. but if you can tell me where is the first point where we start the inproc process, I can put it there as well.

Ah yes, I didn't realize all these get force-connected during us bringing up the instance. This will work (albeit in ugly ways but that's not your fault.)

jasonmalinowski · 2018-01-11T19:30:36Z

@heejaechang Do we need/want this in dev15.6.x so we can land the fix into the VS release branches sooner? I presume they'd want this sooner.

jasonmalinowski · 2018-01-11T19:32:40Z

src/EditorFeatures/Test/Utilities/AsynchronousOperationListenerTests.cs

            {
                var signal = new ManualResetEventSlim();
-                var listener = new Listener();
+                var listener = new AsynchronousOperationListener("test", tracking: false);


Why are these all tracking: false? I don't think it's bad, it just seems odd to be called out directly.

ah, this code is before I made them default parameter. let me remove those.

…or waiter

heejaechang · 2018-01-11T22:03:00Z

retest windows_release_vs-integration_prtest please

heejaechang · 2018-01-11T22:51:12Z

ping?

brettfo

From what I was able to follow,

heejaechang · 2018-01-12T03:14:54Z

there is follow up change in VS side once this is checked in.

https://devdiv.visualstudio.com/DevDiv/_git/VS?path=%2Fsrc%2FOmni%2FApex%2FHostIntegration%2FVisualStudio.OM%2FLanguageServices%2FManaged%2FManagedLanguageService.cs&version=GBlab%2Fvsumain&_a=contents

heejaechang · 2018-01-12T21:57:37Z

@jinujoseph @Pilchie can I get approval for ask mode?

heejaechang · 2018-01-12T21:57:54Z

@jasonmalinowski can you take a look when you have time. I addressed your feedbacks.

Pilchie · 2018-01-16T17:31:24Z

Approved - thanks @heejaechang

heejaechang · 2018-01-16T20:24:49Z

@jasonmalinowski so are you okay with the change? I addressed your PR feedbacks.

…nterface… (dotnet#24120)" This reverts commit 823d973.

* Add UseCultureAttribute to help with culture-dependent unit tests * Disable NuGet package restore in Visual Studio for Roslyn.sln * Add named argument for literal * Fix behavior for NET46 and NETCOREAPP2_0 * Revert "moved waiter from diagnostics.dll to features.dll where all interface… (#24120)" This reverts commit 823d973. * Add references to System.IO.Pipes.AccessControl (#24457) When adding the reference to System.IO.Pipes.AccessControl for the compiler server to use on CoreCLR, I unified the pathway for the desktop and CoreCLR server access control code. This means that System.IO.Pipes.AccessControl needed to be added as a dependent DLL for desktop too, but I forgot to do that. This change adds System.IO.Pipes.AccessControl as a dependent DLL in all the places where the build task is deployed.

…s are defined. (#24512) * put listener change back in (#24120) * leave old types in legacy folder until partner teams move to new interface * added legacy waiter to support partner teams

* Remove duplicate lock DocumentState.s_syntaxTreeToIdMapLock This lock is only being used to protect access to an instance which contains internal synchronization. * Better handle surrounding directives when inlining a local variable. * Add tests. * Share code between VB and C#. * Reduce allocations in UnboundLambda Fixes #23463 * Restore ReturnInferenceCacheKey as the key for _returnInferenceCache * Update code to more closely follow patterns of the original code * Cleanup from code review * basic fix for intellisense in Immediate window * better comments and cleanup * Add basic integration tests * cleanup inproc Immediate window integration test helper * fix incorrect comment * address PR feedback * create Immediate window on ImmediateWindow_InProc.GetText() * Verify MSBuild version in Developer CMD prompt Roslyn is designed to have the simplest possible contribution story: clone then build. Every pre-req needed is either located on the machine or bootstrapped via NuGet. All the way down to using an xcopy MSBuild if needed. The one case which causes a problem is the VS command prompt. In this case MSBuild is pre-installed on the machine and may or may not be suitable for building Roslyn. Previously when building from a VS command prompt we just used whatever MSBuild was provided. The assumption being a developer command prompt was an explicit statement of whath MSBuild you wanted to use. Based on all of our customer reports though this does not seem to be the assumption that consumers of our repo have. The build gave them no explicit errors about the provided toolset and hence when the build failed they assigned flakiness to our repo. Going forward we are applying the same version validation to MSBuild when provided via a developer command prompt. If it doesn't match we will refuse to build asking the user to upgrade VS or build from a normal command prompt. * Remove unneeded debugging line * Comment about pre-release * Added minimum version * Add Omit If Default style option * Add space to be like test without the omit * Add/Remove without needing a property * Reformat * PR feedback * Fix VB diagnostic based on feedback * Handle case of NotApplicable modifier and field declaration list * Fix tests * PR feedback * PR feedback * PreviewCodeAction was overriding ComputeOperations but returning a post-processed operation from original action. This results in another PostProcess being called on the codeaction. If postprocess was overriden in originalaction that'll be ignored the second time (#23920) * Support negative null-check when we are suggesting to inline type checks Fixes #21097 Fixes #24286 * fix a case where persistent storage registration fails and some clean… (#24458) * fix a case where persistent storage registration fails and some clean up code around it. * added readonly * address PR feedback * removed comments no longer relevant * renamed lock name * moved waiter from diagnostics.dll to features.dll where all interfaces are defined. (#24512) * put listener change back in (#24120) * leave old types in legacy folder until partner teams move to new interface * added legacy waiter to support partner teams * Remove methods indirecting access to _metadataFileNameToConvertedProjectReference This field is documented as being written and read from any thread, but in practice all uses are guarded by an AssertIsForeground(). Thus we can get rid of the helper methods that are trying to "help" by locking before accessing the fields, making it really hard to track all the real uses of it. * Make method static that doesn't need state * add a comment to address PR feedback * Fix up tests of P2P to metadata reference conversion It turns out we had some tests, but the tests were disabled. This was because the tests weren't working properly anyways: they were calling into UpdateProjectBinPath which only updated some (but not all) of the project state. That was an internal helper method that shouldn't be used by tests. Updating the tests to use SetBinOutputPathAndRelatedData works better. * Delete debug-only reference validation This was some legacy code that tried to verify that the references we have from the project system match up to what DTE and other sources say. This was debug-only, and the actual asserts were commented out. This is deadweight at this point, so delete it. * added and cleaned up logs around build and live diagnostics. (#24551) also added RoslynActivityLogger that can be enabled through project-system-tool * Avoid closure allocations on the BindSyntaxTreeToId fast path * CS1628 error text mentions in parameters; fixes #24584 * Small cleanup of completion logic. * Move to xunit.console for CoreClr tests Previously we were using xunit.console for desktop tests and dotnet-xunit for our CoreClr tests. This change unifies us on top of xunit.console (now that it has a netcoreapp2.0 version available). * Move unix builds to xunit.runner.console as well * Get actual directory name, not file * Fix dir name issue * fixed build break

* Remove duplicate lock DocumentState.s_syntaxTreeToIdMapLock This lock is only being used to protect access to an instance which contains internal synchronization. * Better handle surrounding directives when inlining a local variable. * Add tests. * Share code between VB and C#. * Reduce allocations in UnboundLambda Fixes #23463 * Restore ReturnInferenceCacheKey as the key for _returnInferenceCache * Update code to more closely follow patterns of the original code * Cleanup from code review * Verify MSBuild version in Developer CMD prompt Roslyn is designed to have the simplest possible contribution story: clone then build. Every pre-req needed is either located on the machine or bootstrapped via NuGet. All the way down to using an xcopy MSBuild if needed. The one case which causes a problem is the VS command prompt. In this case MSBuild is pre-installed on the machine and may or may not be suitable for building Roslyn. Previously when building from a VS command prompt we just used whatever MSBuild was provided. The assumption being a developer command prompt was an explicit statement of whath MSBuild you wanted to use. Based on all of our customer reports though this does not seem to be the assumption that consumers of our repo have. The build gave them no explicit errors about the provided toolset and hence when the build failed they assigned flakiness to our repo. Going forward we are applying the same version validation to MSBuild when provided via a developer command prompt. If it doesn't match we will refuse to build asking the user to upgrade VS or build from a normal command prompt. * Remove unneeded debugging line * Comment about pre-release * Added minimum version * Add Omit If Default style option * Add space to be like test without the omit * Add/Remove without needing a property * Reformat * PR feedback * Fix VB diagnostic based on feedback * Handle case of NotApplicable modifier and field declaration list * Fix tests * PR feedback * PR feedback * Support negative null-check when we are suggesting to inline type checks Fixes #21097 Fixes #24286 * fix a case where persistent storage registration fails and some clean… (#24458) * fix a case where persistent storage registration fails and some clean up code around it. * added readonly * address PR feedback * removed comments no longer relevant * renamed lock name * moved waiter from diagnostics.dll to features.dll where all interfaces are defined. (#24512) * put listener change back in (#24120) * leave old types in legacy folder until partner teams move to new interface * added legacy waiter to support partner teams * Remove methods indirecting access to _metadataFileNameToConvertedProjectReference This field is documented as being written and read from any thread, but in practice all uses are guarded by an AssertIsForeground(). Thus we can get rid of the helper methods that are trying to "help" by locking before accessing the fields, making it really hard to track all the real uses of it. * Make method static that doesn't need state * Fix up tests of P2P to metadata reference conversion It turns out we had some tests, but the tests were disabled. This was because the tests weren't working properly anyways: they were calling into UpdateProjectBinPath which only updated some (but not all) of the project state. That was an internal helper method that shouldn't be used by tests. Updating the tests to use SetBinOutputPathAndRelatedData works better. * Delete debug-only reference validation This was some legacy code that tried to verify that the references we have from the project system match up to what DTE and other sources say. This was debug-only, and the actual asserts were commented out. This is deadweight at this point, so delete it. * added and cleaned up logs around build and live diagnostics. (#24551) also added RoslynActivityLogger that can be enabled through project-system-tool * Avoid closure allocations on the BindSyntaxTreeToId fast path * CS1628 error text mentions in parameters; fixes #24584 * Update optimization data to 2.7.0-beta3-62526-01... * Small cleanup of completion logic. * Locate implementations for reference assemblies using the process binding path * Use GlobalAssemblyCache helper to locate assemblies directly in the GAC * Update InteractiveEditorFeatures to account for a second definition of GlobalAssemblyCache * Move to xunit.console for CoreClr tests Previously we were using xunit.console for desktop tests and dotnet-xunit for our CoreClr tests. This change unifies us on top of xunit.console (now that it has a netcoreapp2.0 version available). * Move unix builds to xunit.runner.console as well * Fixes 559223 Fix and re-enable test that would catch this error * Update LanguageServices training data again... * Get actual directory name, not file * Fix dir name issue * Cleanup based on code review feedback * Check fully-qualified names for SuppressIldasmAttribute and ReferenceAssemblyAttribute * Use correct reference location, or fail decompilation if it's not available * Fix typo... * Don't use inferred member name if that creates duplicates (#24632) * Fixes #23983 * Added test for unique IDEDiagnosticIDs * Fixed capitalization on local variable * Fix `is` and pattern-matching behavior in presence of implicit UD conversion (#24547) * Fix `is` and pattern-matching behavior in presence of implicit UD conversion and also an explicit reference conversion. User-defined conversions should never be considered for `is` and pattern-matching. Fixes #24522

moved waiter from diagnostics.dll to features.dll where all interface…

15aa696

…s are defined.

heejaechang requested a review from a team as a code owner January 9, 2018 11:23

heejaechang added the PR For Personal Review Only The PR doesn’t require anyone other than the developer to review it. label Jan 9, 2018

heejaechang added 2 commits January 9, 2018 03:24

simplified listener and waiter and moved those in product.

eadfef9

cleaned up a bit

cf10f2a

heejaechang commented Jan 9, 2018

View reviewed changes

heejaechang added 11 commits January 9, 2018 12:23

fixed some comments

29f866f

forgot to add new AsyncListenerProvider in test export composition

fd9f159

fixed dead lock found on unit tests

1c472d7

remove all waiters left

6e1de3f

added dispatcher event processing in one more place

cfbed2a

added a way to enable waiter through env variable in test such as DDRIT

27df294

added env var support so that It can be used without internalvisibleT…

38fce31

…o us.

dont wait for all waiters but only onces each test used to wait.

ecc9ad8

removed dependency to Roslyn.Services.Test.Utilities in integration t…

ef186f5

…est. that dll is not in probing path.

fixed test failures

787c356

fixed bug in integration tests

e181c64

move call to enable waiter before creating workspace so that workspac…

9bce4aa

…e event queue get proper async listener

heejaechang commented Jan 11, 2018

View reviewed changes

heejaechang removed the PR For Personal Review Only The PR doesn’t require anyone other than the developer to review it. label Jan 11, 2018

jasonmalinowski reviewed Jan 11, 2018

View reviewed changes

heejaechang added 3 commits January 11, 2018 11:48

expose concret type as well so that it can be directly used in test f…

51dbd31

…or waiter

addressed PR feedbacks.

c28b63b

rename mess up

8941bf7

heejaechang added 2 commits January 11, 2018 12:25

extra ) left out

4f7cc0d

fixed build failure

d9dcb7b

brettfo approved these changes Jan 12, 2018

View reviewed changes

heejaechang changed the base branch from master to dev15.6.x January 12, 2018 23:36

Pilchie added the Approved to merge label Jan 16, 2018

jasonmalinowski approved these changes Jan 17, 2018

View reviewed changes

heejaechang merged commit 823d973 into dotnet:dev15.6.x Jan 17, 2018

jasonmalinowski mentioned this pull request Jan 20, 2018

Audit all asynchronous operations to ensure a waiter is present. #14442

Closed

jasonmalinowski added a commit to jasonmalinowski/roslyn that referenced this pull request Jan 25, 2018

Revert "moved waiter from diagnostics.dll to features.dll where all i…

bd27c0d

…nterface… (dotnet#24120)" This reverts commit 823d973.

heejaechang added a commit to heejaechang/roslyn that referenced this pull request Jan 29, 2018

put listener change back in (dotnet#24120)

f92627f

heejaechang mentioned this pull request Jan 29, 2018

moved waiter from diagnostics.dll to features.dll where all interfaces are defined. #24512

Merged

		@@ -1,16 +0,0 @@
		' Copyright (c) Microsoft. All Rights Reserved. Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

		#If False Then

Conversation

heejaechang commented Jan 9, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Customer scenario

Bugs this fixes

Workarounds, if any

Risk

Performance impact

Is this a regression from a previous update?

Root cause analysis

How was the bug found?

Uh oh!

heejaechang Jan 9, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

heejaechang commented Jan 11, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

heejaechang commented Jan 11, 2018

Uh oh!

heejaechang commented Jan 11, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

brettfo commented Jan 11, 2018

Uh oh!

jasonmalinowski 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!

heejaechang Jan 11, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

heejaechang Jan 11, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

jasonmalinowski commented Jan 11, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

heejaechang commented Jan 9, 2018 •

edited

Loading

heejaechang Jan 9, 2018 •

edited

Loading

heejaechang commented Jan 11, 2018 •

edited

Loading

heejaechang Jan 11, 2018 •

edited

Loading

heejaechang Jan 11, 2018 •

edited

Loading