Enable ros2tpoic hz command #133

yechun1 · 2018-08-14T08:20:00Z

For performance test to print the average publishing rate to screen. Ported rostopic hz command from ros to ros2, based on the file as below

https://github.com/ros/ros_comm/blob/melodic-devel/tools/rostopic/src/rostopic/__init__.py

Signed-off-by: Chris Ye chris.ye@intel.com

Connects to #132

mikaelarguedas

Looks great thanks @yechun1 !

I made a few comments / questions below

mikaelarguedas · 2018-08-15T19:12:38Z

ros2topic/ros2topic/verb/hz.py

@@ -0,0 +1,239 @@
+# Copyright 2017 Open Source Robotics Foundation, Inc.


The vast majority of this code is copied from here the license and copyright should stay the same.

If possible with a comment indicating what version of the file it was copied from (similar to https://github.com/ros2/turtlebot2_demo/blob/2017ddd2c5781434d9dbcb0c172d553e28f7ab02/depthimage_to_pointcloud2/include/depthimage_to_pointcloud2/depth_traits.hpp#L34)

We should also add a license tag with BSD in the package.xml of ros2topic

Stayed with original license, but seems it does not match copyright test.
@pytest.mark.copyright
def test_copyright():
rc = main(argv=['.', 'test'])

assert rc == 0, 'Found errors'

E AssertionError: Found errors
E assert 1 == 0
test/test_copyright.py:23: AssertionError
Captured stderr call
ros2topic/verb/hz.py: copyright=Willow Garage, Inc. (2008), license=
1 errors

mikaelarguedas · 2018-08-15T19:13:11Z

ros2topic/setup.py

@@ -37,6 +37,7 @@
            'info = ros2topic.verb.info:InfoVerb',
            'list = ros2topic.verb.list:ListVerb',
            'pub = ros2topic.verb.pub:PubVerb',
+            'hz = ros2topic.verb.hz:HzVerb',


Nit: alphabetical order

mikaelarguedas · 2018-08-15T19:56:43Z

ros2topic/ros2topic/verb/hz.py

+
+
+class HzVerb(VerbExtension):
+    """print the average publishing rate to screen."""


Nit: print -> Print

mikaelarguedas · 2018-08-16T18:01:03Z

ros2topic/ros2topic/verb/hz.py

+        parser.add_argument(
+            '--window', '-w', default=-1,
+            help='window size, in # of messages, for calculating rate, '
+                 'string to (default: -1)')


As this does not add the --filter and --wall-time arguments, would you mind adding a TODO or opening a ticket to track that these options still need to be ported?

mikaelarguedas · 2018-08-16T20:21:58Z

ros2topic/ros2topic/verb/hz.py

+        :param topic: Topic name
+        """
+        with self.lock:
+            curr = time.time()


It looks like the ROS 1 version of the tool uses ROSTime as the default time source and Walltime only if requested. Is it possible to do the same here ?

There is no simple API similar with rospy.get_rostime (for example rclpy.get_rostime). Added TODO to replace time later. Would you share me the ros2 rostime reference code?

mikaelarguedas · 2018-08-16T20:23:34Z

ros2topic/ros2topic/verb/hz.py

+        """
+        Calculate the average publising rate.
+
+        @returns: tuple of stat results


Nit: please use the same docblock format for all functions

yechun1 · 2018-08-17T09:48:06Z

@mikaelarguedas thanks for your review and comments. Just fixed all those errors and added TODOs. One issue is running test_copyright.py will fail with original copyright. Will it make impact? or do you have suggest how to fix? thanks.

mikaelarguedas · 2018-08-18T02:40:19Z

thanks @yechun1 for iterating.
Could you please submit new commits in the future rather than force-pushing ? this makes reviewing the new changes easier.

Regarding the copyright, it looks like the formatting was diverging from the BSD license template. You can see this commit 8db6c67 for a fir that will make the copyright test pass.

Regarding ROS Time, this is pretty new in rclpy but hopefully you will find some useful information in ros2/rclpy#210

yechun1 · 2018-08-22T07:02:33Z

@mikaelarguedas thanks for your suggestion. I have submitted new commits to fix TODOs. Would you please help to review? thanks.
One item to support multiple topics is not ported, I do not find the way to enable /topic_0 [/topic_1 [topic_2 [..]] to ROS2 topic arguments.

mikaelarguedas

Thanks @yechun1 for adding the missing features!

The code looks good to me with a couple minor comments (inline).

I faced a few inconsistencies when testing this patch for high frequency messages (the reported frequency was always lower but I didn't get a chance to investigate where the bottleneck was). Did you experience the same thing?

Example of commands I used for testing:
PYTHONOPTIMIZED=0 ros2 topic pub /image sensor_msgs/Image -r 1000 -p 1000
PYTHONOPTIMIZED=0 ros2 topic hz /image.

Can you please comment here when you have all the tests passing locally? Someone from @ros2/team will review and rerun Ci.

Thanks!

mikaelarguedas · 2018-08-24T00:11:18Z

ros2topic/ros2topic/verb/hz.py

+    :param topic: topic name, ``list`` of ``str``
+    :param window_size: number of messages to average over, -1 for infinite, ``int``
+    :param filter_expr: Python filter expression that is called with m, the message instance
+    :param blocking: pause hz until topic is published, ``True``


Could you clarify what True refers to here?
It appears that this argument defaults to False, I think the docblock should either refer to the type bool and / or list the default value False.

mikaelarguedas · 2018-08-24T00:13:00Z

ros2topic/ros2topic/verb/hz.py

+    except ValueError as e:
+        raise RuntimeError('Value must be non-negative integer')
+
+    # #694


What is 694 referring too?
Oh it comes from ROS 1. As this doesn't refer to any existing bug tracker I think it would be less confusing to remove it from here and everywhere else in this file.

mikaelarguedas · 2018-08-24T00:21:57Z

ros2topic/ros2topic/verb/hz.py

+    try:
+        window_size = int(args.window)
+    except ValueError as e:
+        raise RuntimeError('Value must be non-negative integer')


It looks like both this check and the implementation accept negative-integers

mikaelarguedas · 2018-08-25T01:43:20Z

ros2topic/ros2topic/verb/hz.py

@@ -0,0 +1,315 @@
+# Software License Agreement (BSD License)


Can you please move line 1 and 2 to line 3 and 4 (as recommended in previous review) this should be enough to fix the linter error (now that ament/ament_lint#106 has been merged)

yechun1 · 2018-08-27T03:01:40Z

A small changes to fix issues by code review. Code has been rebased and all tests passed in my local (by ament test). @ros2/team will you please help review and rerun Ci.

I faced a few inconsistencies when testing this patch for high frequency messages (the reported frequency was always lower but I didn't get a chance to investigate where the bottleneck was). Did you experience the same thing?
Example of commands I used for testing:
PYTHONOPTIMIZED=0 ros2 topic pub /image sensor_msgs/Image -r 1000 -p 1000
PYTHONOPTIMIZED=0 ros2 topic hz /image.

@mikaelarguedas
I think the data reported by hz tool is real. This might be ROS2 performance issue related with pub-sub transport. I have reported similar issue here. I wrote topic hz tool both by python and c++ in my local, found python test result is worse than c++ (even with PYTHONOPTIMIZE), and ros2 hz data is worse than ros, especially for big data transport. Suggest to file another issue about frequency decreases.

yechun1 · 2018-08-28T00:25:15Z

I reproduced the frequency continually reduce issue, I am investigating it.

dirk-thomas · 2018-09-06T17:16:28Z

I reproduced the frequency continually reduce issue, I am investigating it.

@yechun1 What is the status on this?

yechun1 · 2018-09-06T23:23:27Z

this is pending on the publish issue #140. I have not found the root cause.

yechun1 · 2018-09-07T05:24:22Z

The pending Issue #140 have been fixed.

test results is good on my system:

$ ros2 topic pub /Image sensor_msgs/Image -r 1000 -p 1000

$ ros2 topic hz /image -w 1
average rate: 1004.143
min: 0.001s max: 0.001s std dev: 0.00000s window: 1
average rate: 987.825
min: 0.001s max: 0.001s std dev: 0.00000s window: 1
average rate: 1004.864
min: 0.001s max: 0.001s std dev: 0.00000s window: 1

$ ros2 topic hz /image -w 10
average rate: 1000.502
min: 0.001s max: 0.001s std dev: 0.00001s window: 10
average rate: 999.643
min: 0.001s max: 0.001s std dev: 0.00001s window: 10
average rate: 999.953
min: 0.001s max: 0.001s std dev: 0.00001s window: 10

$ ros2 topic hz /image -w 1000
average rate: 999.719
min: 0.000s max: 0.002s std dev: 0.00004s window: 1000
average rate: 999.981
min: 0.000s max: 0.001s std dev: 0.00005s window: 1000
average rate: 1000.011
min: 0.001s max: 0.002s std dev: 0.00004s window: 1000

$ ros2 topic hz /image
average rate: 1000.105
min: 0.000s max: 0.002s std dev: 0.00006s window: 1002
average rate: 1000.055
min: 0.000s max: 0.002s std dev: 0.00006s window: 2003
average rate: 1000.034
min: 0.000s max: 0.002s std dev: 0.00006s window: 3003
average rate: 1000.004
min: 0.000s max: 0.002s std dev: 0.00007s window: 4003
average rate: 1000.009
min: 0.000s max: 0.002s std dev: 0.00006s window: 5004
average rate: 1000.004
min: 0.000s max: 0.002s std dev: 0.00007s window: 6004
average rate: 1000.009
min: 0.000s max: 0.002s std dev: 0.00007s window: 7005
average rate: 1000.005
min: 0.000s max: 0.002s std dev: 0.00007s window: 8005
average rate: 1000.008
min: 0.000s max: 0.002s std dev: 0.00007s window: 9006
average rate: 999.995
min: 0.000s max: 0.002s std dev: 0.00007s window: 10000
average rate: 999.993
min: 0.000s max: 0.002s std dev: 0.00007s window: 10000
average rate: 1000.003
min: 0.000s max: 0.002s std dev: 0.00007s window: 10000

yechun1 · 2018-09-10T07:17:23Z

@dirk-thomas @mikaelarguedas all found issues fixed in this PR. Function verified on my local. Would you please help to review and tell me if there is anything need to change? Thanks.

dirk-thomas · 2018-09-10T16:36:16Z

Please use the same commit strategy as in this PR: #139 (comment)

yechun1 · 2018-09-17T09:10:54Z

no problem, I'll handle this after #139(delay command) been merged. To save rework time.

Copy file from ROS1 and port to ros2. This file is originally from: https://github.com/ros/ros_comm/blob/6e5016f4b2266d8a60c9a1e163c4928b8fc7115e/tools/rostopic/src/rostopic/__init__.py Signed-off-by: Chris Ye <chris.ye@intel.com>

Signed-off-by: Chris Ye <chris.ye@intel.com>

* remove irrelevant code and reserve hz related code * port rostopic hz to ros2topic based on ROS2 API format Signed-off-by: Chris Ye <chris.ye@intel.com>

yechun1 · 2018-10-04T13:32:11Z

squashed the comments and resubmitted again, please help review again.

clalancette

Most of my comments are small nits, so should be easy to fix. The one exception is the comment about using eval, which seems dangerous to me.

clalancette · 2018-10-05T17:15:33Z

ros2topic/ros2topic/verb/hz.py

+            '--window', '-w',
+            dest='window_size', type=unsigned_int, default=DEFAULT_WINDOW_SIZE,
+            help='window size, in # of messages, for calculating rate, '
+                 'string to (default: %d)' % DEFAULT_WINDOW_SIZE, metavar='WINDOW')


Nit: I'm not sure what "string to" means here. Can you reword or remove this piece of the help?

removed. fixed in
0ba39d3

clalancette · 2018-10-05T17:22:21Z

ros2topic/ros2topic/verb/hz.py

+    if args.filter_expr:
+        def expr_eval(expr):
+            def eval_fn(m):
+                return eval(expr)


Hm. I know this is directly ported from the original ROS1 code, but having a naked eval lying around here seems pretty dangerous. In particular, the string that is passed in here can do any arbitrary Python thing, including copying all of your hard drive out to the network, taking command-and-control inputs from the network, or deleting your filesystem. I don't think that sort of power is actually required here; I think a simple regular expression could probably get most of the uses. Could you maybe explain what you think the intended use case here is?

The filter_expr code is directly ported from the original ROS1. No changes from the first commit ("rostopic into ros_comm" ros/ros_comm@a82674a). One comment from code is "#694: ignore messages that don't match filter", line to https://github.com/ros/ros_comm/blob/a82674ad87ab17e373e89d392bcf358c511026ae/tools/rostopic/src/rostopic.py#L116 ", I don't know what #694 means, I guess it is internal bug track of rostopic before open source, and may noted the intended use case there. I have no idea about the requirement of the option --filter. Check with echo command of ros2topic also do not remain this option. Could we also removed here on hz command for code safety?

Yeah, I did some similar code archaeology, and like you said, this option existed before history starts. I do not know the intended use case either. @tfoote, @dirk-thomas , do you have any further history on what the --filter options was meant to achieve in ROS1, and whether we still need it? The options on the table here seem to be:

Remove --filter entirely until a use-case comes up for it.

Restrict --filter to be a regex only.

Leave --filter as the current eval, which seems pretty dangerous.

I'd vote for 1 (we can always add it back later), but I'm interested to hear additional thoughts/opinions.

clalancette · 2018-10-05T17:25:29Z

ros2topic/ros2topic/verb/hz.py

+
+    def __init__(self, node, window_size, filter_expr=None, use_wtime=False):
+        import threading
+        from collections import defaultdict


Nit; looking elsewhere in ros2cli, it looks like we generally hoist defaultdict to the top of the file: https://github.com/ros2/ros2cli/blob/master/ros2cli/ros2cli/entry_points.py#L16 . Mostly a matter of style/taste, just trying to be consistent here.

fixed in
0ba39d3

clalancette · 2018-10-05T17:27:17Z

ros2topic/ros2topic/verb/hz.py

+                return
+
+            curr = curr_rostime.nanoseconds
+            if self.get_msg_t0(topic=topic) < 0 or self.get_msg_t0(topic=topic) > curr:


This would be slightly more efficient as:

msg_t0 = self.get_msg_t0(topic=topic) if msg_t0 < 0 or msg_t0 > curr: ...

thanks, fixed in 0ba39d3

clalancette · 2018-10-05T17:28:30Z

ros2topic/ros2topic/verb/hz.py

+            return
+        with self.lock:
+            # Get frequency every one minute
+            n = len(self.get_times(topic=topic))


Again, probably more efficient to call self.get_times(topic=topic) right after taking the lock, then reuse the local variable.

fixed in 0ba39d3

* remove confused "string to" on help * move import to the top of the file * use local variable instead of multiple funcion call. Signed-off-by: Chris Ye <chris.ye@intel.com>

mjcarroll · 2018-10-08T14:57:01Z

I seem to recall using filter as a convenient way to create a frequency measurement around some value in the message, like if you wanted to measure the frequency of detections, or state changes, or the like.

It is also present on rostopic echo on ROS1.

clalancette · 2018-10-08T15:01:38Z

I seem to recall using filter as a convenient way to create a frequency measurement around some value in the message, like if you wanted to measure the frequency of detections, or state changes, or the like.

It is also present on rostopic echo on ROS1.

Yeah, from the implementation, that would have been my guess as well. If we still think that is useful, that's fine, but I personally think that giving it eval is too powerful. Since it sounds like it might be useful, then I'd think we could get most of its usefulness just by allowing a regex here.

mjcarroll · 2018-10-08T15:04:13Z

Agree that eval is a bit generous in this case, since we haven't ported the echo variant of --filter either, I would be inclined to go with option 1, and then open a feature for reworking that command.

dirk-thomas · 2018-10-08T15:08:41Z

ast.literal_eval could be considered for a replacement.

yechun1 · 2018-10-09T03:51:09Z

ast.literal_eval seems not be satisfied for matching a specified Python expression as ROS 1 provided.
For examples:

$ rostopic echo --filter "m.data=='foo'"  /topic_name
$ rostopic echo --filter "m.transforms[0].child_frame_id == 'my_frame'" /tf
$ rostopic hz --filter "int(m.header.stamp.sec) % 3 == 0" /topic_name

About ast.literal_eval APIs: https://docs.python.org/3/library/ast.html#ast.literal_eval

ast.literal_eval(node_or_string)
Safely evaluate an expression node or a string containing a Python literal or container display. The string or node provided may only consist of the following Python literal structures: strings, bytes, numbers, tuples, lists, dicts, sets, booleans, and None.

This can be used for safely evaluating strings containing Python values from untrusted sources without the need to parse the values oneself. It is not capable of evaluating arbitrarily complex expressions, for example involving operators or indexing.

dirk-thomas · 2018-10-09T05:01:12Z

ast.literal_eval seems not be satisfied for matching a specified Python expression as ROS 1 provided.

Good point. We should continue with option 3 then.

There is no "problem" from a security point of view. The user passes the filter string explicitly as a command line argument. Obviously a "malicious" parameter could be used. But the user could also just call python -c "<malicious command> directly...

yechun1 · 2018-10-11T14:29:21Z

I would like to keep with option 3, as rostopic echo/hz with --filter be used for a long time. Of course, if we have better solution, we could modify the code with new PR.
@clalancette other small issues from your comments fixed in 0ba39d3, would you please help review again if there is anything need to update? Thanks.

clalancette · 2018-10-25T16:52:23Z

I'm going to run CI on this (mostly for the linters); assuming that goes well, I'll merge this later today.

Linux
Linux-aarch64
macOS
Windows

dirk-thomas · 2018-10-25T16:58:18Z

ros2topic/ros2topic/verb/hz.py

+    except ValueError:
+        raise RuntimeError('The passed message type is invalid')
+    module = importlib.import_module(package_name + '.msg')
+    return getattr(module, message_name)


The two methods get_msg_class and _get_msg_class are a 1-to-1 copy from the delay module. Please don't cop-n-paste the code but move it into the api module and reuse it in both locations.

The two methods get_msg_class and _get_msg_class are both used in delay and hz module, avoid cop-n-paste the code but move it into the api module and reuse it in both locations. Signed-off-by: Chris Ye <chris.ye@intel.com>

yechun1 · 2018-10-26T02:43:31Z

Moved get_msg_class and get_msg_class_ from delay and hz modues to api module. Fixed in bbc2293.

clalancette · 2018-10-26T16:10:49Z

All right, one more CI to check the new stuff, then I'll merge.

Linux
Linux-aarch64
macOS
Windows

clalancette · 2018-10-26T18:32:39Z

All the warnings are from other, known problems (they should be fixed now). Given that, I'm going to merge up. Thanks for the contribution.

yechun1 · 2018-10-27T13:30:45Z

@mikaelarguedas @dirk-thomas @clalancette @mjcarroll thank you so much for taking effort on patch review and giving good comments. Glad to see this PR has been merged.

dirk-thomas · 2018-10-27T17:33:56Z

Thank you for making this feature available!

…ros2#133) Signed-off-by: William Woodall <william@osrfoundation.org>

Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com>

tfoote added the in review Waiting for review (Kanban column) label Aug 14, 2018

nuclearsandwich requested a review from mikaelarguedas August 16, 2018 17:02

mikaelarguedas reviewed Aug 16, 2018

View reviewed changes

yechun1 force-pushed the port_rostopic branch 2 times, most recently from e0080ac to 4cf4fa1 Compare August 17, 2018 09:25

yechun1 mentioned this pull request Aug 23, 2018

[ros2topic] enable ros2topic hz delay bw command #132

Closed

3 tasks

mikaelarguedas reviewed Aug 25, 2018

View reviewed changes

yechun1 force-pushed the port_rostopic branch from 26941df to d19461a Compare August 27, 2018 02:27

yechun1 force-pushed the port_rostopic branch from fb030b5 to 06bdaa3 Compare September 1, 2018 11:04

yechun1 mentioned this pull request Sep 1, 2018

[ros2topic] fix pub rate reduced issue #141

Merged

yechun1 mentioned this pull request Sep 6, 2018

[ros2topic] pub frequency lower then expect #140

Closed

dirk-thomas added the requires-changes label Sep 10, 2018

yechun1 force-pushed the port_rostopic branch from f69c469 to fb8082a Compare September 21, 2018 14:42

copy original code for ros2topic hz porting

0559f28

Copy file from ROS1 and port to ros2. This file is originally from: https://github.com/ros/ros_comm/blob/6e5016f4b2266d8a60c9a1e163c4928b8fc7115e/tools/rostopic/src/rostopic/__init__.py Signed-off-by: Chris Ye <chris.ye@intel.com>

yechun1 force-pushed the port_rostopic branch from fb8082a to 198bb9b Compare October 4, 2018 12:38

add ros2topic hz original file link

08659b8

Signed-off-by: Chris Ye <chris.ye@intel.com>

yechun1 force-pushed the port_rostopic branch from 198bb9b to 46a3094 Compare October 4, 2018 13:00

Major function changes for hz cmd porting

5fd3bf9

* remove irrelevant code and reserve hz related code * port rostopic hz to ros2topic based on ROS2 API format Signed-off-by: Chris Ye <chris.ye@intel.com>

yechun1 force-pushed the port_rostopic branch from 46a3094 to 5fd3bf9 Compare October 4, 2018 13:01

clalancette requested changes Oct 5, 2018

View reviewed changes

Small changes to optimize code

0ba39d3

* remove confused "string to" on help * move import to the top of the file * use local variable instead of multiple funcion call. Signed-off-by: Chris Ye <chris.ye@intel.com>

clalancette approved these changes Oct 22, 2018

View reviewed changes

dirk-thomas suggested changes Oct 25, 2018

View reviewed changes

move get_msg_class to API module

bbc2293

The two methods get_msg_class and _get_msg_class are both used in delay and hz module, avoid cop-n-paste the code but move it into the api module and reuse it in both locations. Signed-off-by: Chris Ye <chris.ye@intel.com>

dirk-thomas approved these changes Oct 26, 2018

View reviewed changes

dirk-thomas removed the requires-changes label Oct 26, 2018

clalancette merged commit f609595 into ros2:master Oct 26, 2018

clalancette removed the in review Waiting for review (Kanban column) label Oct 26, 2018

yechun1 deleted the port_rostopic branch October 27, 2018 13:30

esteve pushed a commit to esteve/ros2cli that referenced this pull request Dec 16, 2022

add GroupAction for conditionally including other actions and scoping (…

fe6b296

…ros2#133) Signed-off-by: William Woodall <william@osrfoundation.org>

esteve pushed a commit to esteve/ros2cli that referenced this pull request Dec 16, 2022

Remove unused 'launch' import. (ros2#133)

79bbf06

Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com>

		@@ -0,0 +1,239 @@
		# Copyright 2017 Open Source Robotics Foundation, Inc.



		class HzVerb(VerbExtension):
		"""print the average publishing rate to screen."""

Enable ros2tpoic hz command #133

Enable ros2tpoic hz command #133

Conversation

yechun1 commented Aug 14, 2018 • edited by mikaelarguedas Loading

mikaelarguedas 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

yechun1 commented Aug 17, 2018

mikaelarguedas commented Aug 18, 2018

yechun1 commented Aug 22, 2018

mikaelarguedas 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

yechun1 commented Aug 27, 2018 • edited Loading

yechun1 commented Aug 28, 2018

dirk-thomas commented Sep 6, 2018

yechun1 commented Sep 6, 2018 • edited Loading

yechun1 commented Sep 7, 2018 • edited Loading

yechun1 commented Sep 10, 2018

dirk-thomas commented Sep 10, 2018

yechun1 commented Sep 17, 2018

yechun1 commented Oct 4, 2018

clalancette 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

yechun1 Oct 8, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mjcarroll commented Oct 8, 2018

clalancette commented Oct 8, 2018

mjcarroll commented Oct 8, 2018

dirk-thomas commented Oct 8, 2018

yechun1 commented Oct 9, 2018

dirk-thomas commented Oct 9, 2018

yechun1 commented Oct 11, 2018 • edited Loading

clalancette commented Oct 25, 2018

Choose a reason for hiding this comment

yechun1 commented Oct 26, 2018

clalancette commented Oct 26, 2018

clalancette commented Oct 26, 2018

yechun1 commented Oct 27, 2018

dirk-thomas commented Oct 27, 2018

yechun1 commented Aug 14, 2018 •

edited by mikaelarguedas

Loading

yechun1 commented Aug 27, 2018 •

edited

Loading

yechun1 commented Sep 6, 2018 •

edited

Loading

yechun1 commented Sep 7, 2018 •

edited

Loading

yechun1 Oct 8, 2018 •

edited

Loading

yechun1 commented Oct 11, 2018 •

edited

Loading