Skip to content

Conversation

@shrsr
Copy link
Collaborator

@shrsr shrsr commented Sep 21, 2023

No description provided.

@shrsr shrsr self-assigned this Sep 21, 2023
@codecov
Copy link

codecov bot commented Sep 26, 2023

Codecov Report

Attention: 719 lines in your changes are missing coverage. Please review.

Comparison is base (4cee5c5) 96.62% compared to head (03553da) 35.18%.
Report is 47 commits behind head on master.

❗ Current head 03553da differs from pull request most recent head 870fb1c. Consider uploading reports for the commit 870fb1c to get more accurate results

Files Patch % Lines
plugins/modules/aci_vrf_multicast.py 17.11% 92 Missing ⚠️
plugins/modules/aci_access_switch_policy_group.py 17.00% 83 Missing ⚠️
plugins/module_utils/aci.py 1.69% 58 Missing ⚠️
plugins/modules/aci_l3out_floating_svi_path.py 23.43% 49 Missing ⚠️
plugins/modules/aci_l3out_floating_svi.py 27.27% 40 Missing ⚠️
plugins/modules/aci_fabric_pod_selector.py 33.92% 37 Missing ⚠️
plugins/modules/aci_route_control_context.py 33.33% 32 Missing ⚠️
...odules/aci_l3out_floating_svi_path_secondary_ip.py 36.58% 26 Missing ⚠️
plugins/modules/aci_fabric_wide_settings.py 42.50% 23 Missing ⚠️
plugins/modules/aci_l3out_bgp_protocol_profile.py 41.02% 23 Missing ⚠️
... and 17 more
Additional details and impacted files
@@             Coverage Diff             @@
##           master     #478       +/-   ##
===========================================
- Coverage   96.62%   35.18%   -61.44%     
===========================================
  Files         183      206       +23     
  Lines        8475     9574     +1099     
  Branches     1265     1432      +167     
===========================================
- Hits         8189     3369     -4820     
- Misses        214     6205     +5991     
+ Partials       72        0       -72     
Flag Coverage Δ
integration ?
sanity 35.18% <34.87%> (+0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@shrsr shrsr requested a review from akinross October 3, 2023 21:34
@shrsr
Copy link
Collaborator Author

shrsr commented Oct 4, 2023

Replies-

  1. Addition of l3out Floating SVI module and its child modules #478 (comment)
    -> Not everywhere. See: https://github.com/CiscoDevNet/ansible-aci/blob/0dcf72cdf86875ec818e1790797274175d2786aa/plugins/modules/aci_l3out_logical_interface_profile.py
    However, it might be useful and easier for the user to have more info. I'll go ahead and add those.

  2. Addition of l3out Floating SVI module and its child modules #478 (comment)
    -> I know its been implemented in other modules. However, in this case it would be much easier for customers/users to find the modules when they have appropriate names. In my case the tree structure of the modules looks like this.

  • L3out
  • logical_node_profile
  • logical_interface_profile
  • floating_svi
    • floating_svi_secondary_ip
  • floating_svi_path
    • floating_svi_path_secondary_ip

@shrsr shrsr requested a review from akinross October 4, 2023 19:02
@shrsr shrsr requested a review from akinross October 5, 2023 13:04
akinross
akinross previously approved these changes Oct 5, 2023
Copy link
Collaborator

@akinross akinross left a comment

Choose a reason for hiding this comment

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

LGTM

sajagana
sajagana previously approved these changes Oct 24, 2023
Copy link
Collaborator

@sajagana sajagana left a comment

Choose a reason for hiding this comment

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

LGTM!

gmicol
gmicol previously approved these changes Oct 25, 2023
Copy link
Collaborator

@gmicol gmicol left a comment

Choose a reason for hiding this comment

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

LGTM!!

@shrsr shrsr dismissed stale reviews from gmicol, sajagana, and akinross via 8f78614 November 3, 2023 04:55
akinross
akinross previously approved these changes Nov 3, 2023
Copy link
Collaborator

@akinross akinross left a comment

Choose a reason for hiding this comment

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

LGTM

@shrsr shrsr requested a review from akinross November 7, 2023 18:19
@akinross akinross self-requested a review November 8, 2023 13:31
akinross
akinross previously approved these changes Nov 8, 2023
Copy link
Collaborator

@akinross akinross left a comment

Choose a reason for hiding this comment

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

LGTM

gmicol
gmicol previously approved these changes Nov 8, 2023
Copy link
Collaborator

@gmicol gmicol left a comment

Choose a reason for hiding this comment

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

LGTM!

lhercot
lhercot previously approved these changes Nov 17, 2023
Copy link
Member

@lhercot lhercot left a comment

Choose a reason for hiding this comment

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

LGTM

@lhercot lhercot dismissed stale reviews from gmicol, akinross, and themself via 870fb1c November 17, 2023 01:38
@lhercot lhercot merged commit 4f26255 into CiscoDevNet:master Nov 17, 2023
@gmicol
Copy link
Collaborator

gmicol commented Dec 21, 2023

is part of issue #126

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants