Implemented #320. #339

redboltz · 2019-08-18T20:17:42Z

Added buffer class that supports mqtt::string_view compatible
behavior and life keekping mechanism (optional).

Callback functions for users hold receive buffer directly via
buffer.

Removed *_ref properties. Ref or not ref is hidden by buffer.
Added ""_mb user defined literals for buffer.

Added boost type_erasure.
mqtt::socket is type erased socket. It covers tcp, tls, ws, and wss.
Compile times become faster.

Replaced static_cast with boost::numeric_cast if overflow could happen.
Removed redundant static_cast.

Implemented efficient shared_ptr_array allocation.

If use boost (default), then use boost::make_shared<char[]>(size).
If user defines MQTT_STD_SHARED_PTR_ARRAY,
if __cplusplus is greater than 201703L (means C++20 or later),
then std::make_shared<char[]>(size),
otherwise std::shared_ptr<char[]>(new size)
is called.

Added buffer class that supports `mqtt::string_view` compatible behavior and life keekping mechanism (optional). Callback functions for users hold receive buffer directly via `buffer`. Removed `*_ref` properties. Ref or not ref is hidden by `buffer`. Added `""_mb` user defined literals for `buffer`. Added boost type_erasure. `mqtt::socket` is type erased socket. It covers tcp, tls, ws, and wss. Compile times become faster. Replaced static_cast with boost::numeric_cast if overflow could happen. Removed redundant static_cast. Implemented efficient shared_ptr_array allocation. If use boost (default), then use `boost::make_shared<char[]>(size)`. If user defines MQTT_STD_SHARED_PTR_ARRAY, if __cplusplus is greater than 201703L (means C++20 or later), then `std::make_shared<char[]>(size)`, otherwise `std::shared_ptr<char[]>(new size)` is called.

codecov-io · 2019-08-18T21:51:09Z

Codecov Report

Merging #339 into master will increase coverage by 0.72%.
The diff coverage is 78.63%.

@@            Coverage Diff             @@
##           master     #339      +/-   ##
==========================================
+ Coverage   85.53%   86.25%   +0.72%     
==========================================
  Files          34       37       +3     
  Lines        4889     6215    +1326     
==========================================
+ Hits         4182     5361    +1179     
- Misses        707      854     +147

jonesmz · 2019-08-19T16:47:45Z

.travis.yml

@@ -1,6 +1,6 @@
 sudo: false
 language: cpp
-dist: trusty
+dist: bionic


I'm still cautious about changes to the minimum compiler version.

Did this turn out to still be necessary?

Maybe I can move back to trusty. Would you want to do that?

If it's practical.

If trusty is not "really" too old, and there's an "easy" solution to the compiler issues, then I would prefer to keep supporting trusty, but I'm not opposed to Bionic, as long as the compiler series is still the GCC 7.x.y release.

Like I said, I'm currently stuck on 7.3.0, which supports most of C++17, and it looks like Bionic has GCC 7.4.0, which is probably close enough.

But I'm saying that if there isn't otherwise a reason to switch to the newer version (because compiler errors, for example) then we shouldn't change the minimum version.

Ok, I will downgrade to trusty.

I just confirmed the current version of impl_320_2 works fine with trusty. I will use trusty.

done(just my memo, not pushed yet).

This change does not appear to have been reverted.

Did you revert it?

jonesmz · 2019-08-19T18:32:58Z

example/v5_no_tls_prop.cpp

-        mqtt::v5::property::user_property("key2", "val2"),
-        mqtt::v5::property::authentication_method("test authentication method"),
-        mqtt::v5::property::authentication_data("test authentication data")
+        mqtt::v5::property::user_property("key1"_mb, "val1"_mb),


We could do a user defined literal for UserProperties as well, but it's not clear to me if that is better, or worse, than this approach. Perhaps something that we shouldn't provide at the library level. Or if we did, as a separate header file that people have to include themselves "for convenience".

inline mqtt::v5::property::user_property operator""_user_property(char const* str, std::size_t length) { // Search for separator character (Not sure what character to use. ';' could be one. // Not knowing the right separator character is the reason why i say we probably shouldn't provide this function. // return { mqtt::buffer(string_view of first part), mqtt::buffer(string_view of second part)); }

This PR is already very big and contains many(?) breaking changes. I'd like to consider the issue after the PR merged. Could you create a new issue for that?

I can make an issue.

I'm not sure it's a good idea.

Do you think it's a good idea to offer user-defined literals for all of the property types?

I think that it is good idea for all single string-base properties such as Content Type, Response Topic.
It is not good for binary-base properties such as Correlation Data.

UserProperty is difficult. I think that user defined literals is not sutable for UserProperty. Because separator is required. I think that mqtt::v5::property::user_property("key1"_mb, "val1"_mb) is simple enough.

jonesmz · 2019-08-19T18:43:01Z

include/mqtt/buffer.hpp

@@ -0,0 +1,67 @@
+// Copyright Takatoshi Kondo 2019


For reference, here's the design for a "Shared String", that I use for my own code. (Note: Does not support custom allocators yet. Haven't had time)

Not saying your design is better or worse. Just showing you for sake of sharing design ideas.

I'd be happy to see some of my implementation ideas incorporated into mqtt_cpp, but it's certainly not necessary.

Having mqtt::buffer use a boost::shared_ptr under the hood means that I'm able to write a trivial conversion function from mqtt::buffer -> my "BasicSharedString" code.

https://pastebin.com/2xeqEniX

Thank you for sharing. I will check it later.

Was the BasicSharedString implementation that I shared interesting to you?

jonesmz · 2019-08-19T18:44:41Z

include/mqtt/buffer.hpp

+
+inline namespace literals {
+
+inline buffer operator""_mb(char const* str, std::size_t length) {


Probably put a brief comment here explaining that strings where user defined literals are used have indefinite lifetime in the read-only data section of the binary, and therefore no reference counting is needed.

I will add comments.

done(just my memo, not pushed yet).

jonesmz · 2019-08-19T18:45:33Z

include/mqtt/buffer.hpp

+namespace boost {
+namespace asio {
+
+inline const_buffer buffer(mqtt::buffer const& data) {


A brief comment explaining that management of the mqtt::buffer.lifetime_ needs to happen outside the boost::asio::buffer, e.g. with the "life_keeper" parameters or similar.

I will add the comment.

done(just my memo, not pushed yet).

jonesmz · 2019-08-19T18:46:50Z

include/mqtt/client.hpp

-        base::socket() = std::move(socket);
-        connect_impl(*base::socket(), it, end, std::move(props), std::move(func));
+        socket_ = std::move(socket);
+        base::socket_optional().emplace(socket_);


Why make the socket optional for the base class? That seems weird, that an mqtt::endpoint could ever exist without a socket.

Can the socket be replaced during a session, for example?

It is the most difficult and controversy point of the PR I think.
Let me explain.

The optional is only for lazy initialization. Once the socket is created then it would never be updated.

The design is the same as the current master.

mqtt_cpp/include/mqtt/endpoint.hpp

Lines 7302 to 7308 in 757cba1

/**

* @brief Get Socket shared_ptr reference.

* @return refereence of Socket unique_ptr

*/

std::shared_ptr<Socket>& socket() {

return socket_;

}

mqtt_cpp/include/mqtt/client.hpp

Lines 462 to 475 in 757cba1

void connect(std::vector<v5::property_variant> props, async_handler_t func = async_handler_t()) {

as::ip::tcp::resolver r(ios_);

#if BOOST_VERSION < 106600

as::ip::tcp::resolver::query q(host_, port_);

auto it = r.resolve(q);

as::ip::tcp::resolver::iterator end;

#else // BOOST_VERSION < 106600

auto eps = r.resolve(host_, port_);

auto it = eps.begin();

auto end = eps.end();

#endif // BOOST_VERSION < 106600

setup_socket(base::socket());

connect_impl(*base::socket(), it, end, std::move(props), std::move(func));

}

mqtt_cpp/include/mqtt/client.hpp

Lines 778 to 782 in 757cba1

#if !defined(MQTT_NO_TLS)

template <typename Strand>

void setup_socket(std::shared_ptr<tcp_endpoint<as::ssl::stream<as::ip::tcp::socket>, Strand>>& socket) {

socket.reset(new Socket(ios_, ctx_));

}

On TLS connection, user needs to setup ssl::context before connect. The socket requires ssl::context as the constructor parameter. So the socket should be created in the connect(), not in the client constructor. However, endpoint has the member of shared_ptr<Scoket>, so it is initialized as nullptr at construction time. Then lazy initialized in connect(). This is the original (master) design.

In order to do the same things, I introduced optional in this PR. Because shared_any cannot be initialized by nullptr. Because it requires a reference of the pointee that is hold by the shared_ptr.

That is the reason I introduced optional.

Thank you for the explanation.

I'd rather this change be done separately from the mqtt::buffer changes

But I understand that that is a lot of work to separate the two changes.

Thoughts?

I'd rather this change be done separately from the mqtt::buffer changes
But I understand that that is a lot of work to separate the two changes.

Yes it is difficult. I can't separate them. Because type erasure is required for zero copy buffer without increasing compile times. So I'd like to go this way.

BTW, API socket_optional() is protected. So ordinary user doesn't care about optional.

Can they be separated into two separate commits, on the same PR?

If we keep successful compile result, we can't do that. Because zero copy and not slow compile times required both.

Once I commit slower compile times version, and then modify it, it could be possible but it is hard to me. It is unnecessary work and might take a long time.
So I'd like to commit them in the same commit.

Alright.

I understand.

I'm still not sure what causes the increased compile times, which is why I wanted to see your code by itself, but I understand that it's extra work.

I pushed the slow compile times branch that is not type erased.

It is not continuous to the current impl_320_2 branch but it might help to analyze the reason of compile times increasing.

https://github.com/redboltz/mqtt_cpp/tree/impl_320_no_te_slow_compile

jonesmz · 2019-08-19T20:57:58Z

include/mqtt/endpoint.hpp

+
+    // process common
+
+    templat


would

static constexpr char version_string = { 0x00, 0x04, 'M', 'Q', 'T', 'T' }; memcmp(buf, version_string, sizeof(version_string) != 0

be easier for people to read and understand?

I think that the code reference is something wrong. Could you update it?

Yes. Something is wrong.

I was talking about where you compare the protocol identifier string on the incoming message.

Ok, I will fix it.

done(just my memo, not pushed yet).

jonesmz · 2019-08-19T21:05:51Z

include/mqtt/endpoint.hpp

+
+    // process common
+
+    templat


Not actually using anything captured by reference.

I think that the code reference is something wrong. Could you update it?

Yes. This comment went to the wrong place somehow...

I was pointing out that there are some lambda functions in one of the process_*** functions that are

[&](){...}

But don't actually use any of the variables in the local context. So the & can be removed.

I will fix it.

I noticed that [&] is for this. version_ is referred int the lambda expression.
I know that it is better to write the capture list explicitly but in this case, the lambda expression is defined and called in the same time. Dangling reference can't happen. Only in this case, I think that [&] is acceptable. It is easy to maintain.

Ok. Sounds good.

redboltz · 2019-08-20T00:55:19Z

include/mqtt/endpoint.hpp

+
+    // process common
+
+    templat


I think that the code reference is something wrong. Could you update it?

jonesmz · 2019-08-19T21:09:26Z

include/mqtt/endpoint.hpp

+
+    // process common
+
+    templat


static constexpr

I think that the code reference is something wrong. Could you update it?

There are several

std::size_t size = ...

statements that are inside the "process_***" functions.

Those size variables could be static constexpr

I will fix it.

done(just my memo, not pushed yet).

jonesmz · 2019-08-19T21:14:10Z

include/mqtt/endpoint.hpp

+
+    // process common
+
+    templat


How practical would it be to make all of these process_*** functions into free functions, that accessed the member variables of the endpoint object via the passed in std::shared_ptr<endpoint<>> parameter?

I think that the code reference is something wrong. Could you update it?

The code reference is wrong, but I was talking about process_connect, and process_pingresp and similar.

Some of proces_* functions access buf_ member variable to avoid allocation. It is a private member of endpoint.
For example https://github.com/redboltz/mqtt_cpp/blob/impl_320_2/include/mqtt/endpoint.hpp#L8487-L8505

I think that they should be member function and not practical to become free function. I think that frined function with endpoint parameter is worse than member function.

Ok. I understand.

redboltz · 2019-08-20T00:55:19Z

include/mqtt/endpoint.hpp

+
+    // process common
+
+    templat


I think that the code reference is something wrong. Could you update it?

redboltz · 2019-08-20T00:56:09Z

I believe that I replied all of your comments.

Before I update the code, could you check my comments?

jonesmz · 2019-08-20T03:29:02Z

I think there may have been several places where comments were hidden that you did not see.

I pointed out 2 of them, but there were multiple that I didn't see any response from you.

* Reverted travis-ci linux distro to trusty from bionic. * Added documents for mqtt::buffer. * Added documents for mqtt::shared_ptr_array. * Added documents for mqtt::shared_any. * Removed unused include. * Added mqtt::buffer allocating function. * Replaced `&buf[0]` with `buf.data()` and `&buf_[0]` with `buf_.data()`. * Replaced `buf[0]` with `buf.front()` and `buf_[0]` with `buf_.front()` except continuous `buf[0]` amd `buf[1]` case. * Improved lambda capture on `async_read()`. * Replaced `static_cast` with `boost::numeric_cast` if error could happen. * Added comments for rvalue reference parameters in the local lambda expressions. * Added `calc_variable_length()` to avoid code repeatation. * Replaced `type const val = ...` with `static constexpr type const val = ...`. * Improved MQTT protocol name conparison. * Replaced `mqtt::string_view` passed by const reference with passed by value. * Replaced `buf = std::move(buf).substr(N)` with `buf.remove_prefix(N)`. * Replaecd iterator with plus operator with `std::next()`. * Fixed `topic_name_length_buf_` usage to easy to understand. * Fixed `parse_one()` return value and parameter to passed by reference. * Removed code repeatation from `shared_any`.

redboltz · 2019-08-21T04:08:38Z

I just pushed the commit that fixes all of codes that I commented "done(just my memo, not pushed yet).".

jonesmz · 2019-08-21T04:26:10Z

Ok.

Do you think it's ready to submit now?

Should I go through it one more time?

redboltz · 2019-08-21T04:31:35Z

I believe that it is ready to merge now.
But if you have time, could you check the PR once more?

jonesmz · 2019-08-21T04:33:58Z

I will check it once more right now.

It will take approximately 1 hr.

Can we merge in approximately 1hr, if I don't see any problems?

redboltz · 2019-08-21T04:35:15Z

Sounds good. Thank you very much!

jonesmz · 2019-08-21T04:39:55Z

include/mqtt/shared_ptr_array.hpp

+/**
+ * @brief shared_ptr_array creating function.
+ * You can choose the target type.
+ * - If MQTT_STD_SHARED_PTR_ARRAY is defined,


The CMakeLists.txt file does not offer an option to control this behavior, because this PR does not include changes to CMakeLists.txt

Was that something you planned to do?

I forgot adding MQTT_STD_SHARED_PTR_ARRAY to CMakeLists.txt. I will add the following code to CMakeLists.txt.

OPTION(MQTT_STD_SHARED_PTR_ARRAY "Use std::shared_ptr<char[]> from C++17 instead of boost::shared_ptr<char[]>" OFF) ... IF (MQTT_STD_SHARED_PTR_ARRAY) SET(CMAKE_CXX_STANDARD 17) SET(CMAKE_CXX_STANDARD_REQUIRED ON) MESSAGE (STATUS "Using std::shared_ptr<char []> instead of boost::shared_ptr<char []>. Enables C++17!!!") SET (CMAKE_CXX_FLAGS "-DMQTT_STD_SHARED_PTR_ARRAY ${CMAKE_CXX_FLAGS}") ELSE () MESSAGE (STATUS "Using boost::shared_ptr<char []> instead of boost::shared_ptr<char []>") ENDIF ()

jonesmz · 2019-08-21T04:48:35Z

include/mqtt/v5_message.hpp

-        topic_name_ = as::buffer(&*b, topic_name_length);
-        b += topic_name_length;
+        if (buf.size() < topic_name_length) throw remaining_length_error();
+        utf8string_check(static_cast<mqtt::string_view>(buf).substr(0, topic_name_length));


A comment here explaining why you cast to mqtt::string_view would help future contributors.

In this case, the reason is to avoid increasing the reference count of buf when .substr() is called.

I agree. I will add a comment.

jonesmz · 2019-08-21T05:03:44Z

test/property.cpp

-    BOOST_TEST(v2.val() == v_ref2.val());
-
+BOOST_AUTO_TEST_CASE( content_type ) {
+    mqtt::v5::property::content_type v1 { "abc"_mb };
    BOOST_TEST(boost::lexical_cast<std::string>(v1) == "abc");


I don't understand how boost::lexical_cast knows what to do with mqtt::v5::property::content_type ...?

Can you help me understand?

This is a test for

std::ostream& operator<<(std::ostream&, mqtt::v4::property::content_type const&);

lexical_cast uses stream output operator. https://www.boost.org/doc/libs/1_71_0/doc/html/boost_lexical_cast/synopsis.html#boost_lexical_cast.synopsis.lexical_cast

It is easy to write test codes so I use lexical_cast.

Ah, I did not know that. Thank you for explaining.

jonesmz · 2019-08-21T05:18:41Z

include/mqtt/property.hpp

@@ -915,123 +559,51 @@ class user_property_ref {
    std::size_t num_of_const_buffer_sequence() const {


Can't this function be static constexpr?

Yes, it should be. I will fix it. Including other three member functions.

jonesmz · 2019-08-21T05:21:16Z

include/mqtt/property.hpp

@@ -869,31 +512,32 @@ class user_property_ref {
    void add_const_buffer_sequence(std::vector<as::const_buffer>& v) const {
        v.emplace_back(as::buffer(&id_, 1));
        v.emplace_back(as::buffer(key_.len.data(), key_.len.size()));
-        v.emplace_back(key_.str);
+        v.emplace_back(as::buffer(key_.buf.data(), key_.buf.size()));


We now support

as::buffer(key_.buf)

Ah yes, I will replace it.

jonesmz · 2019-08-21T05:24:33Z

I have reviewed all files except endpoint.hpp.

I am reviewing endpoint.hpp now.

jonesmz · 2019-08-21T05:26:46Z

include/mqtt/endpoint.hpp

+                typename std::iterator_traits<Iterator>::iterator_category,
+                std::random_access_iterator_tag
+            >::value,
+            "Iterator doesn't support random access"


Maybe better wording:

Iterators provided to restore_serialized_message() must be random access iterators.

Thanks, I use your wording.

jonesmz · 2019-08-21T05:27:40Z

include/mqtt/endpoint.hpp

-            restore_serialized_message(basic_pubrel_message<PacketIdBytes>(b, e));
+            restore_serialized_message(
+                basic_pubrel_message<PacketIdBytes>(
+                    buffer(string_view(&*b, static_cast<std::size_t>(std::distance(b, e))))


A comment explaining why you construct the mqtt::buffer such that it does not own the data would be good to have.

I will add the following comment:

// basic_pubrel_message have no member variable that type is mqtt::buffer. // When creating basic_pubrel_message, the constructor just read mqtt::buffer // and convert to some values. // So the argument buffer(...) doesn't need to hold the lifetime.

jonesmz · 2019-08-21T05:42:35Z

include/mqtt/endpoint.hpp

+        async_handler_t func,
+        buffer buf,
+        std::size_t size,
+        std::function<void(buffer, buffer, async_handler_t, this_type_sp)> handler,


Would doing

using process_handler_t = std::function<void(buffer, buffer, async_handler_t, this_type_sp)>;

improve the code?

There are some variations as follows:

std::function<void(packet_id_t, buffer, async_handler_t, this_type_sp)> handler,

std::function<void(buffer, buffer, async_handler_t, this_type_sp)> handler,

So if we introduce type aliases, naming is difficult. I think that current implementation is good enough.

jonesmz · 2019-08-21T05:50:14Z

include/mqtt/endpoint.hpp

+                    call_message_size_error_handlers(func);
+                    return;
+                }
+                if (buf.front() & 0b10000000) {


I think that 0b10000000 should be a named constant.

E.g.

static constexpr std::uint8_t variable_length_flag = 0b10000000

Ok I will introduce the constant.

jonesmz · 2019-08-21T05:51:07Z

include/mqtt/endpoint.hpp

+                    if (!check_error_and_transferred_length(ec, func, bytes_transferred, 1)) return;
+                    proc(
+                        std::move(func),
+                        buffer(string_view(buf_.data(), 1)),


Make a comment saying the lifetime of buf_ is handled by self

I will add comment.

jonesmz · 2019-08-21T05:56:44Z

include/mqtt/endpoint.hpp

+            if (property_length_rest < len) {
+                call_message_size_error_handlers(func);
+                return;
+


Typos.

paramter = parameter
controll = control

I will fix them.

jonesmz · 2019-08-21T06:01:36Z

I've reviewed about 2/3 of endpoint.hpp, and haven't seen any major problems.

I think i'm going to stop now, I ran out of time.

I think this pull request can be merged when you've reviewed my comments from the last hour.

jonesmz · 2019-08-21T06:05:19Z

Also, did you notice that the PR does not think that changes to .travis.yaml were reverted?

redboltz · 2019-08-21T06:54:00Z

Also, did you notice that the PR does not think that changes to .travis.yaml were reverted?

It should be reverted to trusty. I made a mistake to remove that commit. I will fix it.
In addition, add -DMQTT_STD_SHARED_PTR_ARRAY=ON build.

Removed magic number and add constant. Added comments for developer. Updated `num_of_const_buffer_sequence()` from member function to static constexpr function.

I don't know way but codecov upload failed on `pr`.

jonesmz · 2019-08-22T04:13:14Z

Looking good!

redboltz mentioned this pull request Aug 18, 2019

Implemented #320. #337

Closed

jonesmz reviewed Aug 19, 2019

View reviewed changes

redboltz commented Aug 20, 2019

View reviewed changes

include/mqtt/endpoint.hpp

// process common

templat

Copy link

Owner Author

redboltz Aug 20, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that the code reference is something wrong. Could you update it?

jonesmz reviewed Aug 21, 2019

View reviewed changes

Reverted linux distro to trusty from bionic on travis-ci.

5c984ba

redboltz added 6 commits August 21, 2019 15:58

Added MQTT_STD_SHARED_PTR_ARRAY support.

7e38b7d

Reflected review comments.

0ebdbd5

Removed magic number and add constant. Added comments for developer. Updated `num_of_const_buffer_sequence()` from member function to static constexpr function.

Removed codecov retry code.

879177c

Remap osx travis-ci test.

53e3ebd

Comment out travis-ci filter to kick codecov.

80888fd

I don't know way but codecov upload failed on `pr`.

Ballanced osx build on travis-ci.

5a9eb53

Fixed variable length consume calculation.

7047df6

redboltz merged commit f5cb4c5 into master Aug 22, 2019

redboltz deleted the impl_320_2 branch August 22, 2019 11:29


		inline namespace literals {

		inline buffer operator""_mb(char const* str, std::size_t length) {

	/**
	* @brief Get Socket shared_ptr reference.
	* @return refereence of Socket unique_ptr
	*/
	std::shared_ptr<Socket>& socket() {
	return socket_;
	}

	void connect(std::vector<v5::property_variant> props, async_handler_t func = async_handler_t()) {
	as::ip::tcp::resolver r(ios_);
	#if BOOST_VERSION < 106600
	as::ip::tcp::resolver::query q(host_, port_);
	auto it = r.resolve(q);
	as::ip::tcp::resolver::iterator end;
	#else // BOOST_VERSION < 106600
	auto eps = r.resolve(host_, port_);
	auto it = eps.begin();
	auto end = eps.end();
	#endif // BOOST_VERSION < 106600
	setup_socket(base::socket());
	connect_impl(*base::socket(), it, end, std::move(props), std::move(func));
	}

	#if !defined(MQTT_NO_TLS)
	template <typename Strand>
	void setup_socket(std::shared_ptr<tcp_endpoint<as::ssl::stream<as::ip::tcp::socket>, Strand>>& socket) {
	socket.reset(new Socket(ios_, ctx_));
	}

		@@ -915,123 +559,51 @@ class user_property_ref {
		std::size_t num_of_const_buffer_sequence() const {

Implemented #320. #339

Implemented #320. #339

Conversation

redboltz commented Aug 18, 2019

codecov-io commented Aug 18, 2019 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jonesmz Aug 20, 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

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

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

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

jonesmz Aug 19, 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

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

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

Choose a reason for hiding this comment

redboltz commented Aug 20, 2019

jonesmz commented Aug 20, 2019

redboltz commented Aug 21, 2019

jonesmz commented Aug 21, 2019

redboltz commented Aug 21, 2019

jonesmz commented Aug 21, 2019

redboltz commented Aug 21, 2019

jonesmz Aug 21, 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

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

jonesmz commented Aug 21, 2019

Choose a reason for hiding this comment

codecov-io commented Aug 18, 2019 •

edited

Loading

jonesmz Aug 20, 2019 •

edited

Loading

jonesmz Aug 19, 2019 •

edited

Loading

jonesmz Aug 21, 2019 •

edited

Loading