Support merge-include of nested models #659

azeey · 2021-08-06T21:31:35Z

🎉 New feature

Summary

This adds a new attribute, merge, to //include that when set to true copies all the children elements of the nested model into the parent model without introducing a new scope. Currently, we only merge named elements such as //link, //frame , and //joint as well as custom elements. This means we ignore elements like //static and //enable_wind.

For example, given the parent model:

<?xml version="1.0" ?>
<sdf version="1.9">
  <world name="world_model">
    <model name="robot">
      <include merge="true">
        <uri>test_model</uri>
        <pose>100 0 0 0 0 0</pose>
      </include>
    </model>
  </world>
</sdf>

and the included model:

<sdf version="1.9">
  <model name="test_model">
    <link name="L1" />
    <frame name="F1" />
  </model>
</sdf>

The result would be:

<sdf version='1.9'>
  <world name='world_model'>
    <model name='robot'>
      <frame name='test_model__model__' attached_to='L1'>
        <pose relative_to='__model__'>100 0 0 0 -0 0</pose>
      </frame>
      <link name='L1'>
        <pose relative_to='test_model__model__'>0 0 0 0 -0 0</pose>
      </link>
      <frame name='F1' attached_to='test_model__model__'/>
    </model>
  </world>

Test it

Set SDF_PATH to the absolute path of test/integration/model and run ign sdf -p test/integration/merge_include_model.sdf

Checklist

Signed all commits for DCO
Added tests
Added example and/or tutorial
Updated documentation (as needed)
Updated migration guide (as needed)
codecheck passed (See contributing)
All tests passed (See test coverage)
While waiting for a review on your PR, please help review another open pull request to support the maintainers

Note to maintainers: Remember to use Squash-Merge

This change is

Signed-off-by: Addisu Z. Taddese <[email protected]>

EricCousineau-TRI

Reviewable status: 0 of 5 files reviewed, 1 unresolved discussion (waiting on @azeey)

src/parser.cc, line 1584 at r1 (raw file):

            includeSDFFirstElem->SetIncludeElement(includeInfo);
          }
          bool toMerge = _sdf->GetName() == "model" &&

It's not clear to me that this imposes the encapsulation checks.

My preference is that we uphold the invariant that a model should always be valid as a standalone component, and we should actively fail on models that are not fully standalone.

More concretely, a models should always be includable via @merge={false,true}, with the only differences:

@merge=false: Names could now collide; should check.
@merge=true: Names will be nested (only model name could collide w/ existing elements).

Is this upheld by the current code?
If not, is there a (possibly elegant / minimal) solution to uphold encapsulation?

EricCousineau-TRI

Reviewable status: 0 of 5 files reviewed, 1 unresolved discussion (waiting on @azeey)

src/parser.cc, line 1584 at r1 (raw file):

Previously, EricCousineau-TRI (Eric Cousineau) wrote…

It's not clear to me that this imposes the encapsulation checks.

My preference is that we uphold the invariant that a model should always be valid as a standalone component, and we should actively fail on models that are not fully standalone.

More concretely, a models should always be includable via @merge={false,true}, with the only differences:

@merge=false: Names could now collide; should check.

@merge=true: Names will be nested (only model name could collide w/ existing elements).

Is this upheld by the current code?
If not, is there a (possibly elegant / minimal) solution to uphold encapsulation?

More specifically, an included model should not depend on frames, links, joints, etc. that are in the including context.

scpeters

Reviewable status: 0 of 5 files reviewed, 1 unresolved discussion (waiting on @EricCousineau-TRI)

src/parser.cc, line 1584 at r1 (raw file):

Previously, EricCousineau-TRI (Eric Cousineau) wrote…

More specifically, an included model should not depend on frames, links, joints, etc. that are in the including context.

you can construct a throwaway sdf::Model to call the validation functions on a model to be merge-included, but that may be a bit much. Otherwise, the only functions parser.hh that take an sdf::ElementPtr as an input are for checking name uniqueness and for not misusing ::

https://github.com/ignitionrobotics/sdformat/blob/6251baaddea3e3216c09fadb38cc6747b83bf1bf/include/sdf/parser.hh#L409-L428

azeey · 2021-08-09T17:42:29Z

src/parser.cc, line 1584 at r1 (raw file):

Previously, scpeters (Steve Peters) wrote…

you can construct a throwaway sdf::Model to call the validation functions on a model to be merge-included, but that may be a bit much. Otherwise, the only functions parser.hh that take an sdf::ElementPtr as an input are for checking name uniqueness and for not misusing ::

https://github.com/ignitionrobotics/sdformat/blob/6251baaddea3e3216c09fadb38cc6747b83bf1bf/include/sdf/parser.hh#L409-L428

Yeah, since the merging is done in an earlier stage of parsing, I don't think there's an elegant/minimal solution to do the full validation of a model. A less elegant way would be add a __merge__ attribute to the model and add it as a nested model into the parent sdf::ElementPtr and later when sdf::Model is loaded, we perform the actual merge.

ddengster · 2021-08-19T03:59:30Z

Great work @azeey!

I'm looking to override/merge the <cast_shadow> tags of the parent model, do you think we can build on this include-merge feature to do it?

Related issues:
#600
#128

azeey · 2021-08-23T15:06:38Z

I'm looking to override/merge the <cast_shadow> tags of the parent model, do you think we can build on this include-merge feature to do it?

I think that would be out of scope for this PR, but I think the parameter passing feature would work for overriding <cast_shadow>. Have you had a chance to try that?

azeey

Reviewable status: 0 of 5 files reviewed, 1 unresolved discussion (waiting on @azeey)

src/parser.cc, line 1584 at r1 (raw file):

Previously, azeey (Addisu Z. Taddese) wrote…

Yeah, since the merging is done in an earlier stage of parsing, I don't think there's an elegant/minimal solution to do the full validation of a model. A less elegant way would be add a __merge__ attribute to the model and add it as a nested model into the parent sdf::ElementPtr and later when sdf::Model is loaded, we perform the actual merge.

Working: I'll see if using a throwaway sdf::Model would be feasible without becoming a maintenance burden.

ddengster · 2021-08-25T07:39:57Z

I'm looking to override/merge the <cast_shadow> tags of the parent model, do you think we can build on this include-merge feature to do it?

I think that would be out of scope for this PR, but I think the parameter passing feature would work for overriding <cast_shadow>. Have you had a chance to try that?

will try that out, thanks!

Signed-off-by: Addisu Z. Taddese <[email protected]>

codecov-commenter · 2021-08-31T21:45:48Z

Codecov Report

Merging #659 (5d2182a) into main (b6fa291) will increase coverage by 0.11%.
The diff coverage is 91.25%.

❗ Current head 5d2182a differs from pull request most recent head de373d0. Consider uploading reports for the commit de373d0 to get more accurate results

@@            Coverage Diff             @@
##             main     #659      +/-   ##
==========================================
+ Coverage   87.94%   88.06%   +0.11%     
==========================================
  Files          73       75       +2     
  Lines       11170    11333     +163     
==========================================
+ Hits         9824     9980     +156     
- Misses       1346     1353       +7

Impacted Files	Coverage Δ
include/sdf/InterfaceElements.hh	`66.66% <66.66%> (ø)`
src/parser.cc	`85.96% <86.66%> (+0.78%)`	⬆️
src/Utils.cc	`83.20% <92.30%> (+0.39%)`	⬆️
src/Error.cc	`100.00% <100.00%> (ø)`
src/FrameSemantics.cc	`73.87% <100.00%> (ø)`
src/InterfaceElements.cc	`100.00% <100.00%> (ø)`

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b6fa291...de373d0. Read the comment docs.

Signed-off-by: Addisu Z. Taddese <[email protected]>

azeey · 2021-08-31T22:05:45Z

src/parser.cc, line 1584 at r1 (raw file):

Previously, azeey (Addisu Z. Taddese) wrote…

Working: I'll see if using a throwaway sdf::Model would be feasible without becoming a maintenance burden.

Done. Using a throwaway sdf::Model works pretty well. It actually makes it possible to support placement frames when merge=true.

azeey

Reviewable status: 0 of 11 files reviewed, 1 unresolved discussion (waiting on @azeey and @EricCousineau-TRI)

src/parser.cc, line 237 at r3 (raw file):

    ElementPtr proxyModelFrame = _parent->AddElement("frame");
    const std::string proxyModelFrameName = model->Name() + "__model__";

FYI I changed this from __<model name>__model__ to <model name>__model__ because when using the former and generating an SDFormat output file, we end up having a frame with an invalid name since names that start and end with __ are reserved by SDFormat.

scpeters

Reviewed 2 of 5 files at r1, 5 of 8 files at r2, 1 of 3 files at r3, all commit messages.
Reviewable status: 8 of 11 files reviewed, 8 unresolved discussions (waiting on @azeey, @EricCousineau-TRI, and @jennuine)

src/parser.cc, line 210 at r3 (raw file):

      Error unsupportedError(
          ErrorCode::MERGE_INCLUDE_UNSUPPORTED,
          "Merge-include can not supported when parent element is " +

nit: rephrase to "Merge-include does not support parent elements of type " + _parent->GetName()

src/parser.cc, line 237 at r3 (raw file):

    ElementPtr proxyModelFrame = _parent->AddElement("frame");
    const std::string proxyModelFrameName = model->Name() + "__model__";

it's an edge case, but I think this frame name could be invalid if the model name starts with __:

<model name="__valid_model_name">
...
</model>

would give a frame name of __valid_model_name__model__

src/parser.cc, line 333 at r3 (raw file):

      if ((elem->GetName() == "link") || (elem->GetName() == "model") ||
          (elem->GetName() == "joint") || (elem->GetName() == "frame") ||
          (elem->GetName() == "plugin") ||

consider adding "gripper" as well, as I just noticed that it is a named element as well

https://github.com/ignitionrobotics/sdformat/blob/sdformat11_11.2.2/sdf/1.8/model.sdf#L38
https://github.com/ignitionrobotics/sdformat/blob/sdformat11_11.2.2/sdf/1.8/gripper.sdf#L5

src/parser.cc, line 353 at r3 (raw file):

    firstElem->SetParent(_parent);
    _parent->InsertElement(firstElem);
  }

nit: I feel like this function would be a bit more readable if these short else if and else logic blocks came first. Currently I have to scroll past all the merge logic just to see the different branches. Perhaps it could start with:

if (!_merge)
{
  firstElem->SetParent(_parent);
  _parent->InsertElement(firstElem);
  return;
}
else if (firstElem->GetName() == "model")
{
  Error unsupportedError(
      ErrorCode::MERGE_INCLUDE_UNSUPPORTED,
      "Merge-include is only supported for included models");
  _sourceLoc.SetSourceLocationOnError(unsupportedError);
  _errors.push_back(unsupportedError);
  return;
}

then the code from the original if (_merge && firstElem->GetName() == "model") branch can be placed here, with or without a containing else block, since the other branches return;

actually the other error branch can be appended as well as else if (_parent->GetName() != "model")

test/integration/includes.cc, line 496 at r3 (raw file):

{
  sdf::ParserConfig config;
  // Useing the "file://" URI scheme to allow multiple search paths

Useing -> Using

test/integration/model/merge_robot/model.sdf, line 275 at r3 (raw file):


    <joint name='right_wheel_joint' type='revolute'>
      <pose relative_to='__model__'>1 0 0 0 0 0</pose>

there is special logic for handling __model__ in the attributes, but I didn't see any test expectations confirming these values

to be thorough, you could add a //frame with attribute values of __model__ and a joint with //axis/xyz/@expressed_in == "__model__" to test that logic as well

test/sdf/model_invalid_frame_only.sdf, line 5 at r3 (raw file):

  <model name="model_invalid_frame_only">
    <frame name="F1"/>
    <frame name="F2"/>

nit: trailing whitespace on this line

jennuine

Reviewable status: 8 of 11 files reviewed, 12 unresolved discussions (waiting on @azeey and @EricCousineau-TRI)

test/integration/includes.cc, line 591 at r3 (raw file):

    std::stringstream buffer;
    sdf::testing::RedirectConsoleStream redir(
        sdf::Console::Instance()->GetMsgStream(), &buffer);

Would adding this fix the windows CI failure?:
https://github.com/ignitionrobotics/sdformat/blob/a797e18eeb012012f02f194178b16b5bf847fe39/test/integration/deprecated_specs.cc#L77-L84

test/integration/model/merge_robot/model.config, line 4 at r3 (raw file):

<model>
  <name>robot</name>
  <sdf version="1.7">model.sdf</sdf>

1.7 -> 1.9?

test/integration/model/merge_robot/model.sdf, line 2 at r3 (raw file):

<?xml version='1.0' ?>
<sdf version='1.8' xmlns:custom="http://example.org/schema">

1.8 -> 1.9?

test/sdf/model_invalid_frame_only.sdf, line 2 at r3 (raw file):

<?xml version="1.0" ?>
<sdf version='1.7'>

1.7 -> 1.9?

Signed-off-by: Addisu Z. Taddese <[email protected]>

scpeters

Reviewable status: 8 of 11 files reviewed, 8 unresolved discussions (waiting on @azeey, @EricCousineau-TRI, @jennuine, and @scpeters)

test/integration/includes.cc, line 671 at r5 (raw file):

    EXPECT_EQ(5, *errors[0].LineNumber());
    EXPECT_TRUE(buffer.str().find("Error parsing XML in file") !=
                std::string::npos);

suggest adding << buffer.str() between closing ) and ; to aid in debugging the windows test failure

azeey · 2021-09-09T21:08:12Z

a discussion (no related file):
@EricCousineau-TRI I've confirmed that this doesn't work with interface models. My proposed fix is the following:

Add an isMerged flag to sdf::NestedInclude. This would let the custom parser know to parse the model and merge the contents with the parent model. This would also mean Drake would have to be updated to support this.
Add member functions InterfaceLinks, InterfaceFrames, and InterfaceJoints to sdf::Model. Currently only sdf::InterfaceModel has those functions. If an interface model is to be merged, we need these functions in the parent model to access the merged interface elements
Update FrameSemantics.cc to to take into account Interface elements directly under sdf::Model.

Signed-off-by: Addisu Z. Taddese <[email protected]>

azeey · 2021-09-14T22:16:25Z

a discussion (no related file):

Previously, azeey (Addisu Z. Taddese) wrote…

@EricCousineau-TRI I've confirmed that this doesn't work with interface models. My proposed fix is the following:

Add an isMerged flag to sdf::NestedInclude. This would let the custom parser know to parse the model and merge the contents with the parent model. This would also mean Drake would have to be updated to support this.

Add member functions InterfaceLinks, InterfaceFrames, and InterfaceJoints to sdf::Model. Currently only sdf::InterfaceModel has those functions. If an interface model is to be merged, we need these functions in the parent model to access the merged interface elements

Update FrameSemantics.cc to to take into account Interface elements directly under sdf::Model.

Okay, I started working on what I proposed, but I soon realized that it's quite complicated and would need more time to properly test. So, instead, I PIMPLized the sdf::NestedInclude class so that we can fix this after the Fortress release.

Signed-off-by: Addisu Z. Taddese <[email protected]>

azeey

Reviewable status: 7 of 18 files reviewed, 8 unresolved discussions (waiting on @azeey, @EricCousineau-TRI, @jennuine, and @scpeters)

test/integration/includes.cc, line 671 at r5 (raw file):

Previously, scpeters (Steve Peters) wrote…

suggest adding << buffer.str() between closing ) and ; to aid in debugging the windows test failure

Done in e7dc8be.

test/integration/model/merge_robot/model.sdf, line 2 at r3 (raw file):

Previously, scpeters (Steve Peters) wrote…

it looks like sensor_frame is using //pose/@degrees, so I think it should be 1.9

Ah, right. I've updated it to 1.9 in e7dc8be

Signed-off-by: Addisu Z. Taddese <[email protected]>

azeey · 2021-09-16T16:13:32Z

src/parser.cc, line 237 at r3 (raw file):

Previously, azeey (Addisu Z. Taddese) wrote…

That's a good point. I'm not sure how we can avoid that unless we also reserve all names that start with double underscore. Alternatively, we can modify the poses of each element and not use a frame.

How about if I change the pattern to proxy__<model name>__model__?

jennuine

Reviewable status: 7 of 21 files reviewed, 6 unresolved discussions (waiting on @azeey, @EricCousineau-TRI, and @scpeters)

include/sdf/InterfaceElements.hh, line 51 at r8 (raw file):

  // deprecation warnings on memeber variables when simply instantiating this
  // class.
  // TODO(anyone) Remove the constructor and destructor once the depreceted

nit: depreceted -> deprecated

src/InterfaceElements_TEST.cc, line 2 at r8 (raw file):

/*
 * Copyright (C) 2018 Open Source Robotics Foundation

nit: 2018 -> 2021

src/parser.cc, line 237 at r3 (raw file):

Previously, azeey (Addisu Z. Taddese) wrote…

How about if I change the pattern to proxy__<model name>__model__?

or merged__<model name>__model__?

… frame Signed-off-by: Addisu Z. Taddese <[email protected]>

azeey

Reviewable status: 7 of 21 files reviewed, 4 unresolved discussions (waiting on @azeey, @EricCousineau-TRI, and @scpeters)

include/sdf/InterfaceElements.hh, line 51 at r8 (raw file):

Previously, jennuine (Jenn Nguyen) wrote…

nit: depreceted -> deprecated

Done. de373d0

src/InterfaceElements_TEST.cc, line 2 at r8 (raw file):

Previously, jennuine (Jenn Nguyen) wrote…

nit: 2018 -> 2021

Done. de373d0

src/parser.cc, line 237 at r3 (raw file):

Previously, jennuine (Jenn Nguyen) wrote…

or merged__<model name>__model__?

Ok, changed to merged__<model name>__model__ in de373d0

scpeters

Reviewed 2 of 11 files at r6.
Reviewable status: 9 of 21 files reviewed, 1 unresolved discussion (waiting on @azeey and @EricCousineau-TRI)

src/parser.cc, line 237 at r3 (raw file):

Previously, azeey (Addisu Z. Taddese) wrote…

Ok, changed to merged__<model name>__model__ in de373d0

OK

test/integration/model/merge_robot/model.sdf, line 275 at r3 (raw file):

Previously, azeey (Addisu Z. Taddese) wrote…

Done. Added more tests in 923cc72. Instead creating a new joint, I modified the existing one. Let me know if that's okay.

OK

scpeters

Reviewed 5 of 11 files at r6, 1 of 2 files at r7, 3 of 3 files at r8, 3 of 3 files at r9, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @EricCousineau-TRI)

Copied text from gazebosim/sdformat#659. Left a placeholder for an example of decomposing an existing model into separate model files using merge-include. Signed-off-by: Steve Peters <[email protected]>

This proposal suggests a new behavior for the `//model/include` tag that copies and merges the contents of a model file into the current model without adding additional nested model hierarchy or naming scope. The new merging behavior is used when the `//include/@merge` attribute is `true`. Some text for the proposal was copied from the description of the pull request that implemented the majority of this feature (gazebosim/sdformat#659). An example is provided for decomposing a model of a husky with sensors on a pan-tilt gimbal into separate model files using merge-include. Signed-off-by: Eric Cousineau <[email protected]> Signed-off-by: Steve Peters <[email protected]> Co-authored-by: Eric Cousineau <[email protected]>

Prototype implementation of merge-include

1d09b01

Signed-off-by: Addisu Z. Taddese <[email protected]>

azeey requested a review from EricCousineau-TRI August 6, 2021 21:31

github-actions bot added the 🏯 fortress Ignition Fortress label Aug 6, 2021

EricCousineau-TRI reviewed Aug 7, 2021

View reviewed changes

EricCousineau-TRI mentioned this pull request Aug 7, 2021

Merging for Composition: Proposed behavior gazebosim/sdf_tutorials#55

Closed

4 tasks

scpeters reviewed Aug 7, 2021

View reviewed changes

azeey commented Aug 23, 2021

View reviewed changes

azeey added 2 commits August 26, 2021 10:18

Merge remote-tracking branch 'upstream/main' into include_via_merge

d186741

Add more tests

dd3c940

Signed-off-by: Addisu Z. Taddese <[email protected]>

azeey marked this pull request as ready for review August 31, 2021 21:17

Create throwaway sdf::Model only if merge=true

64413f3

Signed-off-by: Addisu Z. Taddese <[email protected]>

azeey self-assigned this Aug 31, 2021

azeey commented Aug 31, 2021

View reviewed changes

azeey requested a review from jennuine August 31, 2021 22:12

scpeters reviewed Sep 2, 2021

View reviewed changes

jennuine reviewed Sep 2, 2021

View reviewed changes

Merge branch 'main' into include_via_merge

ae7978a

chapulina added the beta Targeting beta release of upcoming collection label Sep 8, 2021

azeey added 2 commits September 8, 2021 16:27

Merge remote-tracking branch 'upstream/main' into inclue_merge_rebase

97e0de6

Signed-off-by: Addisu Z. Taddese <[email protected]>

Address reviewer feedback

923cc72

Signed-off-by: Addisu Z. Taddese <[email protected]>

scpeters requested changes Sep 8, 2021

View reviewed changes

azeey added 3 commits September 14, 2021 17:05

Merge remote-tracking branch 'upstream/main' into include_via_merge

db65687

Address reviewer feedback

e7dc8be

Signed-off-by: Addisu Z. Taddese <[email protected]>

PIMPLize sdf::NestedInclude

2a44f68

Signed-off-by: Addisu Z. Taddese <[email protected]>

Add unit test for NestedInclude

c4b7425

Signed-off-by: Addisu Z. Taddese <[email protected]>

azeey commented Sep 14, 2021

View reviewed changes

Fix windows error

5c6bec2

Signed-off-by: Addisu Z. Taddese <[email protected]>

jennuine reviewed Sep 16, 2021

View reviewed changes

azeey added 2 commits September 20, 2021 17:05

Merge remote-tracking branch 'upstream/main' into include_via_merge

03918bd

Fix typos, use merged__<model name>__model__ as the name of the proxy…

de373d0

… frame Signed-off-by: Addisu Z. Taddese <[email protected]>

azeey commented Sep 20, 2021

View reviewed changes

scpeters reviewed Sep 20, 2021

View reviewed changes

scpeters changed the title ~~Add ability to merge included nested models into their parent model (merge-include)~~ Support merge-include of nested models Sep 20, 2021

scpeters approved these changes Sep 20, 2021

View reviewed changes

azeey merged commit 3b4d136 into gazebosim:main Sep 20, 2021

azeey deleted the include_via_merge branch September 20, 2021 23:18

scpeters mentioned this pull request Sep 23, 2021

Prefix merged frames with an underscore #711

Merged

7 tasks

azeey mentioned this pull request Oct 18, 2021

composition: Should provide support for merging / inline models #658

Closed

EricCousineau-TRI mentioned this pull request Jan 12, 2022

sdformat: Should attempt migration from model directives; if sufficient, deprecate model directives RobotLocomotion/drake#15948

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support merge-include of nested models #659

Support merge-include of nested models #659

azeey commented Aug 6, 2021 •

edited

Loading

EricCousineau-TRI left a comment

EricCousineau-TRI left a comment

scpeters left a comment

azeey commented Aug 9, 2021

ddengster commented Aug 19, 2021

azeey commented Aug 23, 2021

azeey left a comment

ddengster commented Aug 25, 2021

codecov-commenter commented Aug 31, 2021 •

edited

Loading

azeey commented Aug 31, 2021

azeey left a comment

scpeters left a comment

jennuine left a comment

scpeters left a comment

azeey commented Sep 9, 2021

azeey commented Sep 14, 2021

azeey left a comment

azeey commented Sep 16, 2021

jennuine left a comment

azeey left a comment

scpeters left a comment

scpeters left a comment

Support merge-include of nested models #659

Support merge-include of nested models #659

Conversation

azeey commented Aug 6, 2021 • edited Loading

🎉 New feature

Summary

Test it

Checklist

EricCousineau-TRI left a comment

Choose a reason for hiding this comment

EricCousineau-TRI left a comment

Choose a reason for hiding this comment

scpeters left a comment

Choose a reason for hiding this comment

azeey commented Aug 9, 2021

ddengster commented Aug 19, 2021

azeey commented Aug 23, 2021

azeey left a comment

Choose a reason for hiding this comment

ddengster commented Aug 25, 2021

codecov-commenter commented Aug 31, 2021 • edited Loading

Codecov Report

azeey commented Aug 31, 2021

azeey left a comment

Choose a reason for hiding this comment

scpeters left a comment

Choose a reason for hiding this comment

jennuine left a comment

Choose a reason for hiding this comment

scpeters left a comment

Choose a reason for hiding this comment

azeey commented Sep 9, 2021

azeey commented Sep 14, 2021

azeey left a comment

Choose a reason for hiding this comment

azeey commented Sep 16, 2021

jennuine left a comment

Choose a reason for hiding this comment

azeey left a comment

Choose a reason for hiding this comment

scpeters left a comment

Choose a reason for hiding this comment

scpeters left a comment

Choose a reason for hiding this comment

azeey commented Aug 6, 2021 •

edited

Loading

codecov-commenter commented Aug 31, 2021 •

edited

Loading