docs(DB): Finish all the database annotations/docs. #237

d-bohls · 2018-03-19T17:58:25Z

Add all DB annotations and docstrings.

Many imports moved into methods to avoid circular imports.

Fixes #48
Fixes #209
Fixes some of #211 (annotations and docs)

BREAKING CHANGE:

Renamed:

Cluster.pd_us_reqd -> Cluster.pdus_reqd
Cluster.sig_refs -> Cluster.sigs
Ecu.clst_ref -> Ecu.clst
Ecu.rx_frm_refs -> Ecu.rx_frms
Ecu.tx_frm_refs -> Ecu.tx_frms
Ecu.linp_2min -> Ecu.lin_p2_min
Ecu.lins_tmin -> Ecu.lin_st_min
Frame.cluster_ref -> Frame.cluster
Frame.sig_refs -> Frame.sigs
Frame.mux_data_mux_sig_ref -> Frame.mux_data_mux_sig
Frame.pdu_refs -> Frame.pdus
LinSched.clst_ref -> LinSched.clst
Pdu.cluster_ref -> Pdu.cluster
Pdu.frm_refs -> Pdu.frms
Pdu.mux_data_mux_sig_ref -> Pdu.mux_data_mux_sig
Pdu.mux_static_sig_refs -> Pdu.mux_static_sigs
Signal.frame_ref -> Signal.frame
Signal.pdu_ref -> Signal.pdu
Signal.mux_subfrm_ref -> Signal.mux_subfrm
SubFrame.frm_ref -> SubFrame.frm
SubFrame.pdu_ref -> SubFrame.pdu

Changed types:

Frame.application_protocol is now constants.AppProtocol
Frame.can_timing_type type is now constants.FrmCanTiming
Frame.lin_checksum type is now constants.FrmLinChecksum
All property renames that dropped "ref" now get/set the db object
except for Cluster.database (opened issue Figure out how to implement the Cluster.database back reference #244)

Removed:

IntfCanFdIsoMode (unused duplicate of CanFdIsoMode)

This contribution adheres to CONTRIBUTING.md.
New tests have been created for any new features or regression tests for bugfixes.
tox successfully runs, including unit tests and style checks (see CONTRIBUTING.md).

coveralls · 2018-03-19T18:04:21Z

Coverage decreased (-0.3%) to 67.763% when pulling b216d61 on d-bohls:dbDocs into 32d7751 on ni:master.

epage · 2018-03-19T18:10:15Z

nixnet/database/_ecu.py

-        return _props.get_ecu_clst_ref(self._handle)
+    def cluster(self):
+        # type: () -> _cluster.Cluster
+        """:any:`Cluster`: Returns the parent cluster to which the ECU is connected.


So it was decided to support back references?

I assumed back references would do the delayed import because I'm assuming that is a less common case. Why is it that the child references are doing the delayed import?

I was unaware that supporting back references was ever in question. I believe they've been there since you added a bunch of properties back in #52. If there is an issue here, please fill me in on details.

I was initially trying to keep most of the delayed imports in Pdu since I heard from Jeff L that they aren't heavily used. After thinking about it more, I like limiting delayed imports to back references. I can also keep Frame, Signal, and SubFrame from having to import Pdu all the time, which is actually what we want if Pdus don't in fact see much usage.

@d-bohls

I was unaware that supporting back references was ever in question.

I first brought up the import issue in #48

74 mostly resolves this for System. The problem is circular imports. For System, I just removed the one place that would require circular imports. For Database, I held off until we decide how to resolve it.

and then just comment that it needs to be addressed in #211.

All other ref properties returning objects. A lot of these have circular imports which
will need to be addressed.

Exposing descendant find (like myFrameA.mySignal). Via getitem, you can get a direct
descendant by name but not indirect descendants. Each parent object
will need an explicit find added to it. This will also run into
problems with circular imports.

I'm still confused. Back references were already in there (since #52), so already supported as I understand. I'm just changing them to return objects instead of refs and addressing the circular imports you mentioned in #211.

The following support is included:

CAN and LIN protocol

Frames, Signals, and frame/signal conversion

Database import

For a complete list of supported features and functions, see the documentation.

From https://github.com/ni/nixnet-python

What got implemented in #52 should not be viewed as "supported" or even usable. I did a quick and dirty implementation of every database property. For example, ref properties were unusable because they just returned integers.

My comments in other issues were to say that there is still design work to be done.

epage · 2018-03-19T18:12:29Z

nixnet/database/_ecu.py

-        _props.set_ecu_rx_frm_refs(self._handle, value)
+    def frames_received(self):
+        # type: () -> typing.Iterable[_frame.Frame]
+        """list of :any:`Frame<_frame.Frame>`: Get or set a list of frames the ECU receives.


Can you explain to us how ECUs and Frames relate? Maybe some pseudo code of how this might look?

I think a received example is also sufficient for transmitted

At the moment, I don't have any information other than the C API documentation here:

http://zone.ni.com/reference/en-XX/help/372841P-01/TOC40.htm

All I've done here is give the existing properties (added in #52) more user-friendly names (what they are called in the linked help) and have them use Frame objects instead of refs.

I'll ping Jeff L and see if he can give us a little info on how these would be used.

@lilesj can you provide any insight as to Ed's question?

An ECU in the database represents a specific node on the BUS or in a sense the physical CAN interface. The ECU item logically maps individual frames to specific ECUs, defining which frames are being transmitted and received by the node. In a fully defined network, each frame in the database will be represented in at least two ECUs. One ECU to transmit and one or more to receive the frame.

An important aspect of the ECU is that it allows you to specify node specific properties which are primarily used by LIN but also used in J1939 over CAN. Examples of node specific properties for LIN are "Master?" and "Protocol Version" where J1939 has a "Node Address" and "Node Name". The driver can then apply these ECU properties to every frame that it transmits to or from the interface.

I put together a little code to set ECU frames:

with database.Database("NIXNET_example") as db: cluster = db.clusters["CAN_Demo_Box_Cluster"] ecu = cluster.ecus["ECU_test"] frames = cluster.frames ecu.frames_transmitted = [frames["SET_BAUD_RATE"], frames["SET_CDB_TRANSMIT"]] ecu.frames_received = [frames["SET_FUNC_GEN_FREQ"], frames["SET_FUNC_GEN_OUTPUT"]] db.save()

That ends up looking like this in the database editor.

epage · 2018-03-19T18:22:44Z

nixnet/database/database.py

+        self.db_needs_closing = False  # To satisfy `__del__` in case nxdb_open_database throws
+
+        if isinstance(database_name, int):
+            # overload the 'database_name' parameter so the Cluster.database property


This looks like an abuse of the parameter. What if a user passes in a number?

One option is to have a keyword-only parameter thats name starts with "_".

Yeah, I wasn't fond of this. I was going to post here and ask for advice but you beat me to it. I wish I could overload the constructor. Instead, I've overloaded the constructor parameter.

With this implementation, if the user passed in a number, the user would get an exception with NX_ERR_DATABASE_BAD_REFERENCE from the driver as soon as they tried to do anything with the object. That seems reasonable to me.

Otherwise, here are my thoughts:

I think I would prefer this functionality be private. The user has no reason to constructor a Database from a ref. It's only done internally for the Cluster.database back reference.

Question: Should the user expect to have to call Database.close on this object? I wouldn't think so, since they may just be doing something like print(Cluster.database.name).

I'll look into the keyword-only parameter idea; it looks promising.

Question: Should the user expect to have to call Database.close on this object? I wouldn't think so, since they may just be doing something like print(Cluster.database.name).

This is multi-part

From the user's perspective, its all the same database reference and I think they shouldn't have to close it but ...

What do we need to do to satisfy the driver (whether it ref counts or not, the use of "close all refs", etc)?

The driver keeps a count where nxdbOpenDatabase increments and nxdbCloseDatabase decrements (unless "close all refs" is true in which case the count goes to 0 and the db is closed).

So we just need to avoid calling nxdbOpenDatabase if we don't expect a matching call to nxdbCloseDatabase (with "close all refs" set to false), which is what I've done here.

Well, I just realized one potential flaw. If you call database.close() twice on the same object, we emit a warning and return early the second time. Now, they can split the calls as follows:

db = Database(some_path)
cluster = db.clusters[cluster_name]
cluster.database.close()
db.close()

I looked into keyword-only parameters but that seems to be only py3 with no backport to py2. I could still use keyword parameters (something like def __init__(self, database_name, **_kwargs): but I'm still not very fond of that, plus it still leaves us with the Database.close() issue I mentioned above.

I looked into having the Cluster constructor take in an existing Database object so it could just pass that back when Cluster.database is called. I had Database pass in a private factory method for Cluster into DbCollection that initializes the Cluster with the Database's self, but then realized properties of other classes (like Ecu.cluster) would also need to know about the Database object when they create a Cluster object.

Another option that I'm not too fond of; what if the Database constructor adds self to a private static list in Database, and Database exposes a private static method to get the Database object that is associated with a given handle? I can imagine a few pitfalls with this approach.

Still researching but I would welcome thoughts on this.

RE private params

What I'd expect is a **kwargs and then we do kwargs.get("_db_id"). nifpga has an example of this though I have no idea why they iterate for the field.

RE Cluster.database

So let's reframe the situation. I can understand doing a db.find to get a subframe and then calling subframe.frame. Is there any context where the Cluster.database is required?

To sum things up, the issue is that for the Database class, we are maintaining state (beyond just the handle itself) to keeps tabs on whether the database needs to be closed. So far, I'm seeing three options:

Use **kwargs to be able to construct a Database using a handle instead of a path. This still suffers from the issue that you have two Database objects that have separate state on whether the database needs to be closed. Perhaps we just don't allow the user to close the database from the back reference object.

Modify all database objects so that they internally store the Database object that was originally used to access them. They all would have a constructor like this: def __init__(self, handle, db): and Cluster.database would return the exact same Database object that was used to access it in the first place, which eliminates the issue of separate state.

Eliminate the back reference Cluster.database. @lilesj do you know of any scenario in which doing this would negatively impact customers?

I reverted Cluster.database for the moment. Perhaps I'll create an issue for it and we can address it later, and not as part of this huge review.

Created issue #244

epage · 2018-03-19T18:23:16Z

nixnet/database/database.py

+            self._handle = database_name
+        else:
+            self._handle = _funcs.nxdb_open_database(database_name)
+            self.db_needs_closing = True


Is it appropriate to make this public?

Nope, my mistake.

epage · 2018-03-19T18:24:44Z

nixnet/_props.py

@@ -2504,13 +2504,13 @@ def get_cluster_pdu_refs(
    )


-def get_cluster_pd_us_reqd(
+def get_cluster_pdus_required(


When I wrote this layer, my intent was for it to reflect the C API and not the python API. If we decide to expand abbreviations, is it appropriate to do so at this level?

Ok, I'll only fix the word splitting.

chlc3d · 2018-03-26T21:40:16Z

nixnet/_cconsts.py

@@ -477,7 +477,7 @@
 NX_PROP_CLST_FRM_REFS = (0x00000004 | NX_CLASS_CLUSTER | NX_PRPTYPE_1_DREF)
 NX_PROP_CLST_NAME = (0x00000005 | NX_CLASS_CLUSTER | NX_PRPTYPE_STRING)
 NX_PROP_CLST_PDU_REFS = (0x00000008 | NX_CLASS_CLUSTER | NX_PRPTYPE_1_DREF)
-NX_PROP_CLST_PD_US_REQD = (0x0000000A | NX_CLASS_CLUSTER | NX_PRPTYPE_BOOL)
+NX_PROP_CLST_PDUS_REQD = (0x0000000A | NX_CLASS_CLUSTER | NX_PRPTYPE_BOOL)
 NX_PROP_CLST_PROTOCOL = (0x00000006 | NX_CLASS_CLUSTER | NX_PRPTYPE_U32)
 NX_PROP_CLST_SIG_REFS = (0x00000007 | NX_CLASS_CLUSTER | NX_PRPTYPE_1_DREF)


Why is sig_ref being changed to signal in some places but not others? Different API layers?

Where is it that you see sig_ref being changed to signal? I'm not finding that.

If you mean, why are we leaving "ref" attached in some places, then yes, it is different API layers. The calls to the driver still deal with references/handles, but we convert those to objects before giving them to the user.

NX_PROP_CLST_SIG_REFS is an internal constant used with driver calls, and it deals with references/handles.

chlc3d · 2018-03-26T23:44:39Z

nixnet/database/_lin_sched_entry.py

-    def collision_res_sched(self, value):
-        _props.set_lin_sched_entry_collision_res_sched(self._handle, value)
+    def collision_resolving_schedule(self):
+        # type: (...) -> typing.Optional[_lin_sched.LinSched]


Why aren't we specifying the input paramaters here? It should just be (), right?

chlc3d · 2018-04-06T20:19:11Z

nixnet/database/_pdu.py

+        # type: () -> int
+        """int: Returns the PDU object configuration status.
+
+        Configuration Status returns an NI-XNET error code.


this will have to be updated in #233 right?

Yes, you are spot on. I expect #233 to go in after this and it should and will be updated with correct annotations/docstrings before it gets pulled.

Thanks for coordinating this guys. Sorry for delay with reviewing, but I'm catching up!

chlc3d · 2018-04-06T20:21:38Z

nixnet/database/_lin_sched_entry.py

+        and the underscore (_) are valid characters for the short name.
+        The space ( ), period (.), and other special characters are not supported within the name.
+        The short name must begin with a letter (uppercase or lowercase) or underscore, and not a number.
+        The short name is limited to 128 characters.


It feels odd to me that this comment lists all the error cases but doesn't tell me what exception gets raised when one of them happens

This is a really good point, and it's a precedent that permeates every C wrapped call. The precedent is that C API calls which return a nxStatus_t error code and are handled by _errors.check_for_error and are covered in docstrings by the C API documentation. When modifying the C API docs for python, we only cover deviations introduced in the python API. Adding driver errors to the python documentation would be a huge effort that would introduce a massively burdensome coupling with very little benefit.

Perhaps I should add, all driver calls that return a nxStatus_t error code go through _errors.check_for_error and raise an XnetError exception when < 0 or emit an XnetWarning if > 0.

That makes sense. Maybe in the future it would be useful to mention that in errors.py or to handle some error in one of the examples to further document it.

chlc3d · 2018-04-06T20:24:30Z

nixnet/database/_lin_sched.py

@@ -43,45 +45,143 @@ def __repr__(self):
        return '{}(handle={})'.format(type(self).__name__, self._handle)

    @property
-    def clst_ref(self):
-        return _props.get_lin_sched_clst_ref(self._handle)
+    def clst(self):


Do we really need to use names like "clst" and "frm" instead of "cluster" and "frame"? It feels weird to me that we're using different names from the C properties but only going halfway toward making them clear.

Something I should have done when we started the project but never did was write up a style guide to explain this. Hopefully @jashnani is working on it?

My original intent

Naming in the bindings maps directly to C (minus PEP8-ifying them)

The user-facing API reuses the existing C API abbreviations assuming those were chosen for a reason

Deviate in naming when we are doing something different. For example, we are no longer returning a cluster reference / handle but instead a cluster object, so the suffix no longer is needed.

My first push of this PR did attempt to rename things like frm to frame and clst to cluster, but @epage and @jashnani requested that we not deviate from the C API at this point in time. This is for the reasons @epage just mentioned. I would definitely like to see this naming revisited in the near future.

I'm working on an API style guide for the Python that will cover naming. If we're going to deviate from the C API, it'll be in accordance to that guide, so we know when and how to deviate and are consistent in doing so across the package.

jashnani

Good job overall! Documentation and type checks looks solid. I like that now property getters return type Enum as opposed to the underlying datatype - it matches the setters and makes overall experience consistent.

I have some minor questions/comments on your code.

jashnani · 2018-03-30T21:19:46Z

nixnet/_enums.py

-    ISO = _cconsts.NX_CAN_FD_MODE_ISO
-    NON_ISO = _cconsts.NX_CAN_FD_MODE_NON_ISO
-    ISO_LEGACY = _cconsts.NX_CAN_FD_MODE_ISO_LEGACY
-


Why is this class being deleted?

From the PR comment:

Removed:
IntfCanFdIsoMode (unused duplicate of CanFdIsoMode)

jashnani · 2018-04-13T16:58:20Z

nixnet/database/_frame.py

+        Within your application,
+        change this property to the desired timing type.
+        """
+        return constants.FrmCanTiming(_props.get_frame_can_timing_type(self._handle))


Where was TimeType coming from before?

It wasn't coming from anywhere, so this was a bug.

Oh wow, good catch!

jashnani · 2018-04-13T17:04:27Z

nixnet/database/_lin_sched.py

@@ -43,45 +45,143 @@ def __repr__(self):
        return '{}(handle={})'.format(type(self).__name__, self._handle)

    @property
-    def clst_ref(self):
-        return _props.get_lin_sched_clst_ref(self._handle)
+    def clst(self):


I'm working on an API style guide for the Python that will cover naming. If we're going to deviate from the C API, it'll be in accordance to that guide, so we know when and how to deviate and are consistent in doing so across the package.

jashnani · 2018-04-13T17:06:44Z

nixnet/database/_lin_sched.py

+        """
+        handle = _props.get_lin_sched_clst_ref(self._handle)
+        cluster = _cluster.Cluster(handle)
+        return cluster


super minor: you can just return after you've created the Cluster.
return _cluster.Cluster(handle)

jashnani · 2018-04-13T17:11:05Z

nixnet/database/_pdu.py

+        # type: () -> int
+        """int: Returns the PDU object configuration status.
+
+        Configuration Status returns an NI-XNET error code.


Thanks for coordinating this guys. Sorry for delay with reviewing, but I'm catching up!

jashnani · 2018-04-13T17:32:48Z

nixnet/database/_frame.py

        ref = _props.get_frame_mux_data_mux_sig_ref(self._handle)
        if ref == 0:
            # A bit of an abuse of errors
            _errors.check_for_error(_cconsts.NX_ERR_SIGNAL_NOT_FOUND)
-        return ref
+        return _signal.Signal(ref)


In some cases, based on the value of ref or handle, we're raising exceptions or returning None but in other cases we don't have this check. What was your criteria for determining when to raise errors, return None or do nothing?

Is the error reporting deviating from the C API in a meaningful way?

Here are the cases:

If the property can return a null ref from the C API, it is called out in the C API documentation. For python in this case, when the ref is null, we raise an exception and call that exception out in the Python docstrings.

If the property will never return a null ref from the C API, the C API documentation will exclude any mention of returning a null. For example, a frame will always have a cluster parent, so frame.cluster will always return a non-zero ref. For python in this case, there is no need to raise an exception or mention anything in the docstrings.

We decided against returning None where refs are concerned. I found I'd missed a case in LinSchedEntry that still returned None, but I've just now fixed that to raise an exception.

jashnani · 2018-04-13T18:05:27Z

nixnet/database/_lin_sched_entry.py

+        if not handle:
+            return None
+        lin_sched = _lin_sched.LinSched(handle)
+        return lin_sched


In some cases we're raising exceptions or returning None but in other cases we don't have this check. What was your criteria for determining when to check for a bad handle?

Ah, you also found these case I'd missed that returns None. As noted in the above comment, I've just fixed this to raise an exception.

Good catch btw.

Add all DB annotations and docstrings. Many imports moved into methods to avoid circular imports. Fixes ni#48 Fixes ni#209 Fixes some of ni#211 (annotations and docs) BREAKING CHANGE: Renamed: * Cluster.pd_us_reqd -> Cluster.pdus_reqd * Cluster.sig_refs -> Cluster.sigs * Ecu.clst_ref -> Ecu.clst * Ecu.rx_frm_refs -> Ecu.rx_frms * Ecu.tx_frm_refs -> Ecu.tx_frms * Ecu.linp_2min -> Ecu.lin_p2_min * Ecu.lins_tmin -> Ecu.lin_st_min * Frame.cluster_ref -> Frame.cluster * Frame.sig_refs -> Frame.sigs * Frame.mux_data_mux_sig_ref -> Frame.mux_data_mux_sig * Frame.pdu_refs -> Frame.pdus * LinSched.clst_ref -> LinSched.clst * Pdu.cluster_ref -> Pdu.cluster * Pdu.frm_refs -> Pdu.frms * Pdu.mux_data_mux_sig_ref -> Pdu.mux_data_mux_sig * Pdu.mux_static_sig_refs -> Pdu.mux_static_sigs * Signal.frame_ref -> Signal.frame * Signal.pdu_ref -> Signal.pdu * Signal.mux_subfrm_ref -> Signal.mux_subfrm * SubFrame.frm_ref -> SubFrame.frm * SubFrame.pdu_ref -> SubFrame.pdu Changed types: * Frame.application_protocol is now constants.AppProtocol * Frame.can_timing_type type is now constants.FrmCanTiming * Frame.lin_checksum type is now constants.FrmLinChecksum * All property renames that dropped "ref" now get/set the db object except for Cluster.database (opened issue ni#244) Removed: * IntfCanFdIsoMode (unused duplicate of CanFdIsoMode)

jashnani

Ship it!

d-bohls self-assigned this Mar 19, 2018

d-bohls requested a review from jashnani March 19, 2018 17:58

epage reviewed Mar 19, 2018

View reviewed changes

d-bohls mentioned this pull request Mar 21, 2018

Support for rest of the database API (non-RT) #211

Open

14 tasks

d-bohls force-pushed the dbDocs branch from 6addee4 to dc2f505 Compare March 26, 2018 16:51

d-bohls mentioned this pull request Mar 27, 2018

Figure out how to implement the Cluster.database back reference #244

Open

d-bohls force-pushed the dbDocs branch 6 times, most recently from abf6da7 to 365693a Compare March 28, 2018 21:00

d-bohls added the breaking-change label Apr 4, 2018

d-bohls mentioned this pull request Apr 6, 2018

Frame.pdu_* properties should be considered for a tuple #245

Merged

3 tasks

chlc3d reviewed Apr 6, 2018

View reviewed changes

d-bohls mentioned this pull request Apr 11, 2018

feat(API): Add descendant find for parent db objects. #248

Merged

3 tasks

d-bohls force-pushed the dbDocs branch from 365693a to 68b6ae1 Compare April 11, 2018 21:37

jashnani reviewed Apr 13, 2018

View reviewed changes

d-bohls force-pushed the dbDocs branch from 68b6ae1 to e92e08b Compare April 13, 2018 20:08

d-bohls force-pushed the dbDocs branch from e92e08b to b216d61 Compare April 13, 2018 20:35

jashnani self-requested a review April 13, 2018 20:43

jashnani approved these changes Apr 13, 2018

View reviewed changes

jashnani merged commit e60323c into ni:master Apr 13, 2018

d-bohls deleted the dbDocs branch April 13, 2018 23:19

docs(DB): Finish all the database annotations/docs. #237

docs(DB): Finish all the database annotations/docs. #237

Conversation

d-bohls commented Mar 19, 2018 • edited Loading

coveralls commented Mar 19, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

d-bohls Mar 20, 2018 • 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

d-bohls Mar 19, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

d-bohls Apr 11, 2018 • 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

d-bohls Apr 7, 2018 • edited Loading

Choose a reason for hiding this comment

d-bohls Apr 7, 2018 • 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

d-bohls Apr 7, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jashnani left a comment • 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

jashnani left a comment

Choose a reason for hiding this comment

d-bohls commented Mar 19, 2018 •

edited

Loading

coveralls commented Mar 19, 2018 •

edited

Loading

d-bohls Mar 20, 2018 •

edited

Loading

d-bohls Mar 19, 2018 •

edited

Loading

d-bohls Apr 11, 2018 •

edited

Loading

d-bohls Apr 7, 2018 •

edited

Loading

d-bohls Apr 7, 2018 •

edited

Loading

d-bohls Apr 7, 2018 •

edited

Loading

jashnani left a comment •

edited

Loading